From e4eb82b7382a9ef110fe9c6f970d26eae9394c13 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 16 Nov 2024 15:51:59 +0100 Subject: [PATCH] fix: use better code to group UID and stopwatches - Instead of having code that relied on the result being sorted (which wasn't specified in the query and therefore not safe to assume so). Use a map where it doesn't care if the result that we get from the database is sorted or not. - Added unit test. --- .../TestGetUIDsAndStopwatch/stopwatch.yml | 11 +++++ models/issues/stopwatch.go | 23 ++--------- models/issues/stopwatch_test.go | 40 +++++++++++++++++++ modules/eventsource/manager_run.go | 6 +-- 4 files changed, 58 insertions(+), 22 deletions(-) create mode 100644 models/issues/TestGetUIDsAndStopwatch/stopwatch.yml diff --git a/models/issues/TestGetUIDsAndStopwatch/stopwatch.yml b/models/issues/TestGetUIDsAndStopwatch/stopwatch.yml new file mode 100644 index 0000000000..f564e4b389 --- /dev/null +++ b/models/issues/TestGetUIDsAndStopwatch/stopwatch.yml @@ -0,0 +1,11 @@ +- + id: 3 + user_id: 1 + issue_id: 2 + created_unix: 1500988004 + +- + id: 4 + user_id: 3 + issue_id: 0 + created_unix: 1500988003 diff --git a/models/issues/stopwatch.go b/models/issues/stopwatch.go index fd9c7d7875..93eaf8845d 100644 --- a/models/issues/stopwatch.go +++ b/models/issues/stopwatch.go @@ -60,34 +60,19 @@ func getStopwatch(ctx context.Context, userID, issueID int64) (sw *Stopwatch, ex return sw, exists, err } -// UserIDCount is a simple coalition of UserID and Count -type UserStopwatch struct { - UserID int64 - StopWatches []*Stopwatch -} - // GetUIDsAndNotificationCounts between the two provided times -func GetUIDsAndStopwatch(ctx context.Context) ([]*UserStopwatch, error) { +func GetUIDsAndStopwatch(ctx context.Context) (map[int64][]*Stopwatch, error) { sws := []*Stopwatch{} if err := db.GetEngine(ctx).Where("issue_id != 0").Find(&sws); err != nil { return nil, err } + res := map[int64][]*Stopwatch{} if len(sws) == 0 { - return []*UserStopwatch{}, nil + return res, nil } - lastUserID := int64(-1) - res := []*UserStopwatch{} for _, sw := range sws { - if lastUserID == sw.UserID { - lastUserStopwatch := res[len(res)-1] - lastUserStopwatch.StopWatches = append(lastUserStopwatch.StopWatches, sw) - } else { - res = append(res, &UserStopwatch{ - UserID: sw.UserID, - StopWatches: []*Stopwatch{sw}, - }) - } + res[sw.UserID] = append(res[sw.UserID], sw) } return res, nil } diff --git a/models/issues/stopwatch_test.go b/models/issues/stopwatch_test.go index 68a11acd96..af86e8b1d8 100644 --- a/models/issues/stopwatch_test.go +++ b/models/issues/stopwatch_test.go @@ -4,12 +4,14 @@ package issues_test import ( + "path/filepath" "testing" "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "github.com/stretchr/testify/assert" @@ -77,3 +79,41 @@ func TestCreateOrStopIssueStopwatch(t *testing.T) { unittest.AssertNotExistsBean(t, &issues_model.Stopwatch{UserID: 2, IssueID: 2}) unittest.AssertExistsAndLoadBean(t, &issues_model.TrackedTime{UserID: 2, IssueID: 2}) } + +func TestGetUIDsAndStopwatch(t *testing.T) { + defer unittest.OverrideFixtures( + unittest.FixturesOptions{ + Dir: filepath.Join(setting.AppWorkPath, "models/fixtures/"), + Base: setting.AppWorkPath, + Dirs: []string{"models/issues/TestGetUIDsAndStopwatch/"}, + }, + )() + require.NoError(t, unittest.PrepareTestDatabase()) + + uidStopwatches, err := issues_model.GetUIDsAndStopwatch(db.DefaultContext) + require.NoError(t, err) + assert.EqualValues(t, map[int64][]*issues_model.Stopwatch{ + 1: { + { + ID: 1, + UserID: 1, + IssueID: 1, + CreatedUnix: timeutil.TimeStamp(1500988001), + }, + { + ID: 3, + UserID: 1, + IssueID: 2, + CreatedUnix: timeutil.TimeStamp(1500988004), + }, + }, + 2: { + { + ID: 2, + UserID: 2, + IssueID: 2, + CreatedUnix: timeutil.TimeStamp(1500988002), + }, + }, + }, uidStopwatches) +} diff --git a/modules/eventsource/manager_run.go b/modules/eventsource/manager_run.go index f66dc78c7e..d4d95ae72a 100644 --- a/modules/eventsource/manager_run.go +++ b/modules/eventsource/manager_run.go @@ -90,8 +90,8 @@ loop: return } - for _, userStopwatches := range usersStopwatches { - apiSWs, err := convert.ToStopWatches(ctx, userStopwatches.StopWatches) + for uid, stopwatches := range usersStopwatches { + apiSWs, err := convert.ToStopWatches(ctx, stopwatches) if err != nil { if !issues_model.IsErrIssueNotExist(err) { log.Error("Unable to APIFormat stopwatches: %v", err) @@ -103,7 +103,7 @@ loop: log.Error("Unable to marshal stopwatches: %v", err) continue } - m.SendMessage(userStopwatches.UserID, &Event{ + m.SendMessage(uid, &Event{ Name: "stopwatches", Data: string(dataBs), })