domi/forgejo-domi
1
0
Fork 0
forked from forgejo/forgejo
Code Pull requests Activity

[MODERATION] Refactor excluding watchers mechanism (squash)

Browse source 
This solves two bugs. One bug is that due to the JOIN with the
`forgejo_blocked_users` table, duplicated users were generated if a user
had more than one user blocked, this lead to receiving more than one
entry in the actions table. The other bug is that if a user blocked more
than one user, it would still receive a action entry by a
blocked user, because the SQL query would not exclude the other
duplicated users that was generated by the JOIN.

The new solution is somewhat non-optimal in my eyes, but it's better
than rewriting the query to become a potential perfomance blocker (usage
of WHERE IN, which cannot be rewritten to a JOIN). It simply removes the
watchers after it was retrieved by the SQL query.
This commit is contained in:
Gusted 2024-01-15 00:22:06 +01:00
parent cd1eadd923
commit c63c00b39b
No known key found for this signature in database
GPG key ID: FD821B732837125F
5 changed files with 87 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

19
models/activities/action.go
View file

@ -9,6 +9,7 @@ import (
"fmt"
"net/url"
"path"
"slices"
"strconv"
"strings"
"time"
@ -21,6 +22,7 @@ import (
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
@ -591,10 +593,25 @@ func NotifyWatchers(ctx context.Context, actions ...*Action) error {
if repoChanged {
// Add feeds for user self and all watchers.
watchers, err = repo_model.GetWatchersExcludeBlocked(ctx, act.RepoID, act.ActUserID)
watchers, err = repo_model.GetWatchers(ctx, act.RepoID)
if err != nil {
return fmt.Errorf("get watchers: %w", err)
}
// Be aware that optimizing this correctly into the `GetWatchers` SQL
// query is for most cases less performant than doing this.
blockedDoerUserIDs, err := user_model.ListBlockedByUsersID(ctx, act.ActUserID)
if err != nil {
return fmt.Errorf("user_model.ListBlockedByUsersID: %w", err)
}
if len(blockedDoerUserIDs) > 0 {
excludeWatcherIDs := make(container.Set[int64], len(blockedDoerUserIDs))
excludeWatcherIDs.AddMultiple(blockedDoerUserIDs...)
watchers = slices.DeleteFunc(watchers, func(v *repo_model.Watch) bool {
return excludeWatcherIDs.Contains(v.UserID)
})
}
}
// Add feed for actioner.