From 4aae7929a8500107937de31b6394f4195b30afb3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 16 Nov 2024 11:41:01 -0800 Subject: [PATCH] Fix test --- models/activities/action_list.go | 52 ++++++++++ models/activities/action_test.go | 149 ---------------------------- routers/api/v1/repo/repo.go | 2 +- services/feed/feed.go | 54 +--------- services/feed/feed_test.go | 165 +++++++++++++++++++++++++++++++ 5 files changed, 220 insertions(+), 202 deletions(-) create mode 100644 services/feed/feed_test.go diff --git a/models/activities/action_list.go b/models/activities/action_list.go index aafb7f8a26..5f9acb8f2a 100644 --- a/models/activities/action_list.go +++ b/models/activities/action_list.go @@ -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 +} diff --git a/models/activities/action_test.go b/models/activities/action_test.go index e5dee33ae0..4d4645988b 100644 --- a/models/activities/action_test.go +++ b/models/activities/action_test.go @@ -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 @@ -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.") @@ -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) -} diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 8895a79180..40990a28cb 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -34,9 +34,9 @@ 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" - feed_service "code.gitea.io/gitea/services/feed" ) // Search repositories via options diff --git a/services/feed/feed.go b/services/feed/feed.go index fa00c6cf2d..93bf875fd0 100644 --- a/services/feed/feed.go +++ b/services/feed/feed.go @@ -5,61 +5,11 @@ package feed import ( "context" - "fmt" - "code.gitea.io/gitea/models/activities" activities_model "code.gitea.io/gitea/models/activities" - "code.gitea.io/gitea/models/db" ) // GetFeeds returns actions according to the provided options -func GetFeeds(ctx context.Context, opts activities_model.GetFeedsOptions) (activities.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 := activities_model.ActivityQueryCondition(ctx, opts) - if err != nil { - return nil, 0, err - } - - actions := make([]*activities_model.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 := activities_model.ActionList(actions).LoadAttributes(ctx); err != nil { - return nil, 0, fmt.Errorf("LoadAttributes: %w", err) - } - - return actions, count, nil +func GetFeeds(ctx context.Context, opts activities_model.GetFeedsOptions) (activities_model.ActionList, int64, error) { + return activities_model.GetFeeds(ctx, opts) } diff --git a/services/feed/feed_test.go b/services/feed/feed_test.go new file mode 100644 index 0000000000..6f1cb9a969 --- /dev/null +++ b/services/feed/feed_test.go @@ -0,0 +1,165 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package feed + +import ( + "testing" + + activities_model "code.gitea.io/gitea/models/activities" + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" +) + +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 := 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 = 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 := 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 = 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 = 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 = 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 := 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 = 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 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 := 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 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 := GetFeeds(db.DefaultContext, activities_model.GetFeedsOptions{ + RequestedRepo: repo, + }) + assert.NoError(t, err) + assert.Len(t, actions, 1) +}