Skip to content

Commit

Permalink
Merge remote-tracking branch 'giteaofficial/main'
Browse files Browse the repository at this point in the history
* giteaofficial/main:
  Split mail sender sub package from mailer service package (go-gitea#32618)
  Fix a bug in actions artifact test (go-gitea#32672)
  Move GetFeeds to service layer (go-gitea#32526)
  • Loading branch information
zjjhot committed Nov 30, 2024
2 parents 7ad51ab + 79d593a commit 24e46aa
Show file tree
Hide file tree
Showing 32 changed files with 758 additions and 621 deletions.
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/private/mail.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/mailer"
sender_service "code.gitea.io/gitea/services/mailer/sender"
)

// SendEmail pushes messages to mail queue
Expand Down Expand Up @@ -81,7 +82,7 @@ func SendEmail(ctx *context.PrivateContext) {

func sendEmail(ctx *context.PrivateContext, subject, message string, to []string) {
for _, email := range to {
msg := mailer.NewMessage(email, subject, message)
msg := sender_service.NewMessage(email, subject, message)
mailer.SendAsync(msg)
}

Expand Down
Loading

0 comments on commit 24e46aa

Please sign in to comment.