Skip to content
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

Move GetFeeds to service layer #32526

Merged
merged 10 commits into from
Nov 29, 2024
54 changes: 1 addition & 53 deletions models/activities/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,65 +448,13 @@ type GetFeedsOptions struct {
Date string // the day we want activity for: YYYY-MM-DD
}

// GetFeeds returns actions according to the provided options
func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, error) {
if opts.RequestedUser == nil && opts.RequestedTeam == nil && opts.RequestedRepo == nil {
return nil, 0, fmt.Errorf("need at least one of these filters: RequestedUser, RequestedTeam, RequestedRepo")
}

cond, err := activityQueryCondition(ctx, opts)
if err != nil {
return nil, 0, err
}

actions := make([]*Action, 0, opts.PageSize)
var count int64
opts.SetDefaultValues()

if opts.Page < 10 { // TODO: why it's 10 but other values? It's an experience value.
sess := db.GetEngine(ctx).Where(cond)
sess = db.SetSessionPagination(sess, &opts)

count, err = sess.Desc("`action`.created_unix").FindAndCount(&actions)
if err != nil {
return nil, 0, fmt.Errorf("FindAndCount: %w", err)
}
} else {
// First, only query which IDs are necessary, and only then query all actions to speed up the overall query
sess := db.GetEngine(ctx).Where(cond).Select("`action`.id")
sess = db.SetSessionPagination(sess, &opts)

actionIDs := make([]int64, 0, opts.PageSize)
if err := sess.Table("action").Desc("`action`.created_unix").Find(&actionIDs); err != nil {
return nil, 0, fmt.Errorf("Find(actionsIDs): %w", err)
}

count, err = db.GetEngine(ctx).Where(cond).
Table("action").
Cols("`action`.id").Count()
if err != nil {
return nil, 0, fmt.Errorf("Count: %w", err)
}

if err := db.GetEngine(ctx).In("`action`.id", actionIDs).Desc("`action`.created_unix").Find(&actions); err != nil {
return nil, 0, fmt.Errorf("Find: %w", err)
}
}

if err := ActionList(actions).LoadAttributes(ctx); err != nil {
return nil, 0, fmt.Errorf("LoadAttributes: %w", err)
}

return actions, count, nil
}

// ActivityReadable return whether doer can read activities of user
func ActivityReadable(user, doer *user_model.User) bool {
return !user.KeepActivityPrivate ||
doer != nil && (doer.IsAdmin || user.ID == doer.ID)
}

func activityQueryCondition(ctx context.Context, opts GetFeedsOptions) (builder.Cond, error) {
func ActivityQueryCondition(ctx context.Context, opts GetFeedsOptions) (builder.Cond, error) {
cond := builder.NewCond()

if opts.RequestedTeam != nil && opts.RequestedUser == nil {
Expand Down
52 changes: 52 additions & 0 deletions models/activities/action_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,3 +201,55 @@ func (actions ActionList) LoadIssues(ctx context.Context) error {
}
return nil
}

// GetFeeds returns actions according to the provided options
func GetFeeds(ctx context.Context, opts GetFeedsOptions) (ActionList, int64, error) {
if opts.RequestedUser == nil && opts.RequestedTeam == nil && opts.RequestedRepo == nil {
return nil, 0, fmt.Errorf("need at least one of these filters: RequestedUser, RequestedTeam, RequestedRepo")
}

cond, err := ActivityQueryCondition(ctx, opts)
if err != nil {
return nil, 0, err
}

actions := make([]*Action, 0, opts.PageSize)
var count int64
opts.SetDefaultValues()

if opts.Page < 10 { // TODO: why it's 10 but other values? It's an experience value.
sess := db.GetEngine(ctx).Where(cond)
sess = db.SetSessionPagination(sess, &opts)

count, err = sess.Desc("`action`.created_unix").FindAndCount(&actions)
if err != nil {
return nil, 0, fmt.Errorf("FindAndCount: %w", err)
}
} else {
// First, only query which IDs are necessary, and only then query all actions to speed up the overall query
sess := db.GetEngine(ctx).Where(cond).Select("`action`.id")
sess = db.SetSessionPagination(sess, &opts)

actionIDs := make([]int64, 0, opts.PageSize)
if err := sess.Table("action").Desc("`action`.created_unix").Find(&actionIDs); err != nil {
return nil, 0, fmt.Errorf("Find(actionsIDs): %w", err)
}

count, err = db.GetEngine(ctx).Where(cond).
Table("action").
Cols("`action`.id").Count()
if err != nil {
return nil, 0, fmt.Errorf("Count: %w", err)
}

if err := db.GetEngine(ctx).In("`action`.id", actionIDs).Desc("`action`.created_unix").Find(&actions); err != nil {
return nil, 0, fmt.Errorf("Find: %w", err)
}
}

if err := ActionList(actions).LoadAttributes(ctx); err != nil {
return nil, 0, fmt.Errorf("LoadAttributes: %w", err)
}

return actions, count, nil
}
149 changes: 0 additions & 149 deletions models/activities/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,114 +42,6 @@ func TestAction_GetRepoLink(t *testing.T) {
assert.Equal(t, comment.HTMLURL(db.DefaultContext), action.GetCommentHTMLURL(db.DefaultContext))
}

func TestGetFeeds(t *testing.T) {
// test with an individual user
assert.NoError(t, unittest.PrepareTestDatabase())
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})

actions, count, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
RequestedUser: user,
Actor: user,
IncludePrivate: true,
OnlyPerformedBy: false,
IncludeDeleted: true,
})
assert.NoError(t, err)
if assert.Len(t, actions, 1) {
assert.EqualValues(t, 1, actions[0].ID)
assert.EqualValues(t, user.ID, actions[0].UserID)
}
assert.Equal(t, int64(1), count)

actions, count, err = activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
RequestedUser: user,
Actor: user,
IncludePrivate: false,
OnlyPerformedBy: false,
})
assert.NoError(t, err)
assert.Len(t, actions, 0)
assert.Equal(t, int64(0), count)
}

func TestGetFeedsForRepos(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
privRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
pubRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 8})

// private repo & no login
actions, count, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
RequestedRepo: privRepo,
IncludePrivate: true,
})
assert.NoError(t, err)
assert.Len(t, actions, 0)
assert.Equal(t, int64(0), count)

// public repo & no login
actions, count, err = activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
RequestedRepo: pubRepo,
IncludePrivate: true,
})
assert.NoError(t, err)
assert.Len(t, actions, 1)
assert.Equal(t, int64(1), count)

// private repo and login
actions, count, err = activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
RequestedRepo: privRepo,
IncludePrivate: true,
Actor: user,
})
assert.NoError(t, err)
assert.Len(t, actions, 1)
assert.Equal(t, int64(1), count)

// public repo & login
actions, count, err = activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
RequestedRepo: pubRepo,
IncludePrivate: true,
Actor: user,
})
assert.NoError(t, err)
assert.Len(t, actions, 1)
assert.Equal(t, int64(1), count)
}

func TestGetFeeds2(t *testing.T) {
// test with an organization user
assert.NoError(t, unittest.PrepareTestDatabase())
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})

actions, count, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
RequestedUser: org,
Actor: user,
IncludePrivate: true,
OnlyPerformedBy: false,
IncludeDeleted: true,
})
assert.NoError(t, err)
assert.Len(t, actions, 1)
if assert.Len(t, actions, 1) {
assert.EqualValues(t, 2, actions[0].ID)
assert.EqualValues(t, org.ID, actions[0].UserID)
}
assert.Equal(t, int64(1), count)

actions, count, err = activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
RequestedUser: org,
Actor: user,
IncludePrivate: false,
OnlyPerformedBy: false,
IncludeDeleted: true,
})
assert.NoError(t, err)
assert.Len(t, actions, 0)
assert.Equal(t, int64(0), count)
}

func TestActivityReadable(t *testing.T) {
tt := []struct {
desc string
Expand Down Expand Up @@ -227,26 +119,6 @@ func TestNotifyWatchers(t *testing.T) {
})
}

func TestGetFeedsCorrupted(t *testing.T) {
// Now we will not check for corrupted data in the feeds
// users should run doctor to fix their data
assert.NoError(t, unittest.PrepareTestDatabase())
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
unittest.AssertExistsAndLoadBean(t, &activities_model.Action{
ID: 8,
RepoID: 1700,
})

actions, count, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
RequestedUser: user,
Actor: user,
IncludePrivate: true,
})
assert.NoError(t, err)
assert.Len(t, actions, 1)
assert.Equal(t, int64(1), count)
}

func TestConsistencyUpdateAction(t *testing.T) {
if !setting.Database.Type.IsSQLite3() {
t.Skip("Test is only for SQLite database.")
Expand Down Expand Up @@ -322,24 +194,3 @@ func TestDeleteIssueActions(t *testing.T) {
assert.NoError(t, activities_model.DeleteIssueActions(db.DefaultContext, issue.RepoID, issue.ID, issue.Index))
unittest.AssertCount(t, &activities_model.Action{}, 0)
}

func TestRepoActions(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
_ = db.TruncateBeans(db.DefaultContext, &activities_model.Action{})
for i := 0; i < 3; i++ {
_ = db.Insert(db.DefaultContext, &activities_model.Action{
UserID: 2 + int64(i),
ActUserID: 2,
RepoID: repo.ID,
OpType: activities_model.ActionCommentIssue,
})
}
count, _ := db.Count[activities_model.Action](db.DefaultContext, &db.ListOptions{})
assert.EqualValues(t, 3, count)
actions, _, err := activities_model.GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{
RequestedRepo: repo,
})
assert.NoError(t, err)
assert.Len(t, actions, 1)
}
2 changes: 1 addition & 1 deletion models/activities/user_heatmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func getUserHeatmapData(ctx context.Context, user *user_model.User, team *organi
groupByName = groupBy
}

cond, err := activityQueryCondition(ctx, GetFeedsOptions{
cond, err := ActivityQueryCondition(ctx, GetFeedsOptions{
RequestedUser: user,
RequestedTeam: team,
Actor: doer,
Expand Down
3 changes: 2 additions & 1 deletion routers/api/v1/org/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/routers/api/v1/utils"
"code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/convert"
feed_service "code.gitea.io/gitea/services/feed"
"code.gitea.io/gitea/services/org"
user_service "code.gitea.io/gitea/services/user"
)
Expand Down Expand Up @@ -447,7 +448,7 @@ func ListOrgActivityFeeds(ctx *context.APIContext) {
ListOptions: listOptions,
}

feeds, count, err := activities_model.GetFeeds(ctx, opts)
feeds, count, err := feed_service.GetFeeds(ctx, opts)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetFeeds", err)
return
Expand Down
3 changes: 2 additions & 1 deletion routers/api/v1/org/team.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"code.gitea.io/gitea/routers/api/v1/utils"
"code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/convert"
feed_service "code.gitea.io/gitea/services/feed"
org_service "code.gitea.io/gitea/services/org"
repo_service "code.gitea.io/gitea/services/repository"
)
Expand Down Expand Up @@ -882,7 +883,7 @@ func ListTeamActivityFeeds(ctx *context.APIContext) {
ListOptions: listOptions,
}

feeds, count, err := activities_model.GetFeeds(ctx, opts)
feeds, count, err := feed_service.GetFeeds(ctx, opts)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetFeeds", err)
return
Expand Down
3 changes: 2 additions & 1 deletion routers/api/v1/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
actions_service "code.gitea.io/gitea/services/actions"
"code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/convert"
feed_service "code.gitea.io/gitea/services/feed"
"code.gitea.io/gitea/services/issue"
repo_service "code.gitea.io/gitea/services/repository"
)
Expand Down Expand Up @@ -1313,7 +1314,7 @@ func ListRepoActivityFeeds(ctx *context.APIContext) {
ListOptions: listOptions,
}

feeds, count, err := activities_model.GetFeeds(ctx, opts)
feeds, count, err := feed_service.GetFeeds(ctx, opts)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetFeeds", err)
return
Expand Down
3 changes: 2 additions & 1 deletion routers/api/v1/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"code.gitea.io/gitea/routers/api/v1/utils"
"code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/convert"
feed_service "code.gitea.io/gitea/services/feed"
)

// Search search users
Expand Down Expand Up @@ -214,7 +215,7 @@ func ListUserActivityFeeds(ctx *context.APIContext) {
ListOptions: listOptions,
}

feeds, count, err := activities_model.GetFeeds(ctx, opts)
feeds, count, err := feed_service.GetFeeds(ctx, opts)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetFeeds", err)
return
Expand Down
3 changes: 2 additions & 1 deletion routers/web/feed/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"code.gitea.io/gitea/models/renderhelper"
"code.gitea.io/gitea/modules/markup/markdown"
"code.gitea.io/gitea/services/context"
feed_service "code.gitea.io/gitea/services/feed"

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

actions, _, err := activities_model.GetFeeds(ctx, activities_model.GetFeedsOptions{
actions, _, err := feed_service.GetFeeds(ctx, activities_model.GetFeedsOptions{
RequestedUser: ctx.ContextUser,
Actor: ctx.Doer,
IncludePrivate: includePrivate,
Expand Down
3 changes: 2 additions & 1 deletion routers/web/feed/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import (
activities_model "code.gitea.io/gitea/models/activities"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/services/context"
feed_service "code.gitea.io/gitea/services/feed"

"github.com/gorilla/feeds"
)

// ShowRepoFeed shows user activity on the repo as RSS / Atom feed
func ShowRepoFeed(ctx *context.Context, repo *repo_model.Repository, formatType string) {
actions, _, err := activities_model.GetFeeds(ctx, activities_model.GetFeedsOptions{
actions, _, err := feed_service.GetFeeds(ctx, activities_model.GetFeedsOptions{
RequestedRepo: repo,
Actor: ctx.Doer,
IncludePrivate: true,
Expand Down
Loading
Loading