diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index eba33a7c36369..3eff5789f32cd 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -26,8 +26,6 @@ avatar: avatar1 avatar_email: user1@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 0 @@ -63,8 +61,6 @@ avatar: avatar2 avatar_email: user2@example.com use_custom_avatar: false - num_followers: 2 - num_following: 1 num_stars: 2 num_repos: 13 num_teams: 0 @@ -100,8 +96,6 @@ avatar: avatar3 avatar_email: user3@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 3 num_teams: 5 @@ -137,8 +131,6 @@ avatar: avatar4 avatar_email: user4@example.com use_custom_avatar: false - num_followers: 0 - num_following: 1 num_stars: 0 num_repos: 0 num_teams: 0 @@ -174,8 +166,6 @@ avatar: avatar5 avatar_email: user5@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 1 num_teams: 0 @@ -211,8 +201,6 @@ avatar: avatar6 avatar_email: user6@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 2 @@ -248,8 +236,6 @@ avatar: avatar7 avatar_email: user7@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 1 @@ -285,8 +271,6 @@ avatar: avatar8 avatar_email: user8@example.com use_custom_avatar: false - num_followers: 1 - num_following: 1 num_stars: 0 num_repos: 0 num_teams: 0 @@ -322,8 +306,6 @@ avatar: avatar9 avatar_email: user9@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 0 @@ -359,8 +341,6 @@ avatar: avatar10 avatar_email: user10@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 3 num_teams: 0 @@ -396,8 +376,6 @@ avatar: avatar11 avatar_email: user11@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 1 num_teams: 0 @@ -433,8 +411,6 @@ avatar: avatar12 avatar_email: user12@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 1 num_teams: 0 @@ -470,8 +446,6 @@ avatar: avatar13 avatar_email: user13@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 1 num_teams: 0 @@ -507,8 +481,6 @@ avatar: avatar14 avatar_email: user13@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 3 num_teams: 0 @@ -544,8 +516,6 @@ avatar: avatar15 avatar_email: user15@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 4 num_teams: 0 @@ -581,8 +551,6 @@ avatar: avatar16 avatar_email: user16@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 2 num_teams: 0 @@ -618,8 +586,6 @@ avatar: avatar17 avatar_email: user17@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 2 num_teams: 3 @@ -655,8 +621,6 @@ avatar: avatar18 avatar_email: user18@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 0 @@ -692,8 +656,6 @@ avatar: avatar19 avatar_email: user19@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 2 num_teams: 1 @@ -729,8 +691,6 @@ avatar: avatar20 avatar_email: user20@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 4 num_teams: 0 @@ -766,8 +726,6 @@ avatar: avatar21 avatar_email: user21@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 2 num_teams: 0 @@ -803,8 +761,6 @@ avatar: avatar22 avatar_email: limited_org@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 2 num_teams: 1 @@ -840,8 +796,6 @@ avatar: avatar23 avatar_email: privated_org@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 2 num_teams: 2 @@ -877,8 +831,6 @@ avatar: avatar24 avatar_email: user24@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 0 @@ -914,8 +866,6 @@ avatar: avatar25 avatar_email: org25@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 1 @@ -951,8 +901,6 @@ avatar: avatar26 avatar_email: org26@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 4 num_teams: 1 @@ -988,8 +936,6 @@ avatar: avatar27 avatar_email: user27@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 3 num_teams: 0 @@ -1025,8 +971,6 @@ avatar: avatar28 avatar_email: user28@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 0 @@ -1062,8 +1006,6 @@ avatar: avatar29 avatar_email: user29@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 0 @@ -1099,8 +1041,6 @@ avatar: avatar29 avatar_email: user30@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 4 num_teams: 0 @@ -1136,8 +1076,6 @@ avatar: avatar31 avatar_email: user31@example.com use_custom_avatar: false - num_followers: 0 - num_following: 1 num_stars: 0 num_repos: 0 num_teams: 0 @@ -1173,8 +1111,6 @@ avatar: avatar32 avatar_email: user30@example.com use_custom_avatar: false - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 0 @@ -1210,8 +1146,6 @@ avatar: avatar33 avatar_email: user33@example.com use_custom_avatar: false - num_followers: 1 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 0 @@ -1248,8 +1182,6 @@ avatar: avatar34 avatar_email: user34@example.com use_custom_avatar: true - num_followers: 0 - num_following: 0 num_stars: 0 num_repos: 0 num_teams: 0 diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 26fe1dea129c3..0fddc7d2be951 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -494,15 +494,17 @@ var migrations = []Migration{ NewMigration("Add is_internal column to package", v1_20.AddIsInternalColumnToPackage), // v257 -> v258 NewMigration("Add Actions Artifact table", v1_20.CreateActionArtifactTable), - // v258 -> 259 + // v258 -> v259 NewMigration("Add PinOrder Column", v1_20.AddPinOrderToIssue), - // v259 -> 260 + // v259 -> v260 NewMigration("Convert scoped access tokens", v1_20.ConvertScopedAccessTokens), // Gitea 1.20.0 ends at 260 // v260 -> v261 NewMigration("Drop custom_labels column of action_runner table", v1_21.DropCustomLabelsColumnOfActionRunner), + // v261 -> v262 + NewMigration("Drop column NumFollowers and NumFollowing on User", v1_21.DropColumnNumFollowersAndNumFollowingOnUser), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_21/v261.go b/models/migrations/v1_21/v261.go new file mode 100644 index 0000000000000..ee855baeec3ad --- /dev/null +++ b/models/migrations/v1_21/v261.go @@ -0,0 +1,22 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_21 //nolint + +import ( + "code.gitea.io/gitea/models/migrations/base" + + "xorm.io/xorm" +) + +func DropColumnNumFollowersAndNumFollowingOnUser(x *xorm.Engine) error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + if err := base.DropTableColumns(sess, "user", "num_followers", "num_following"); err != nil { + return err + } + return sess.Commit() +} diff --git a/models/unittest/consistency.go b/models/unittest/consistency.go index 41798c62536f5..4fb2b8dd68886 100644 --- a/models/unittest/consistency.go +++ b/models/unittest/consistency.go @@ -69,8 +69,6 @@ func init() { AssertCountByCond(t, "star", builder.Eq{"uid": user.int("ID")}, user.int("NumStars")) AssertCountByCond(t, "org_user", builder.Eq{"org_id": user.int("ID")}, user.int("NumMembers")) AssertCountByCond(t, "team", builder.Eq{"org_id": user.int("ID")}, user.int("NumTeams")) - AssertCountByCond(t, "follow", builder.Eq{"user_id": user.int("ID")}, user.int("NumFollowing")) - AssertCountByCond(t, "follow", builder.Eq{"follow_id": user.int("ID")}, user.int("NumFollowers")) if user.int("Type") != modelsUserTypeOrganization { assert.EqualValues(t, 0, user.int("NumMembers"), "Unexpected number of members for user id: %d", user.int("ID")) assert.EqualValues(t, 0, user.int("NumTeams"), "Unexpected number of teams for user id: %d", user.int("ID")) diff --git a/models/user/follow.go b/models/user/follow.go index 7efecc26a781b..b565c01b8e025 100644 --- a/models/user/follow.go +++ b/models/user/follow.go @@ -41,14 +41,6 @@ func FollowUser(userID, followID int64) (err error) { if err = db.Insert(ctx, &Follow{UserID: userID, FollowID: followID}); err != nil { return err } - - if _, err = db.Exec(ctx, "UPDATE `user` SET num_followers = num_followers + 1 WHERE id = ?", followID); err != nil { - return err - } - - if _, err = db.Exec(ctx, "UPDATE `user` SET num_following = num_following + 1 WHERE id = ?", userID); err != nil { - return err - } return committer.Commit() } @@ -67,13 +59,5 @@ func UnfollowUser(userID, followID int64) (err error) { if _, err = db.DeleteByBean(ctx, &Follow{UserID: userID, FollowID: followID}); err != nil { return err } - - if _, err = db.Exec(ctx, "UPDATE `user` SET num_followers = num_followers - 1 WHERE id = ?", followID); err != nil { - return err - } - - if _, err = db.Exec(ctx, "UPDATE `user` SET num_following = num_following - 1 WHERE id = ?", userID); err != nil { - return err - } return committer.Commit() } diff --git a/models/user/user.go b/models/user/user.go index 2077d55f513e0..03a3ba1affdd1 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -30,6 +30,7 @@ import ( "code.gitea.io/gitea/modules/validation" "xorm.io/builder" + "xorm.io/xorm" ) // UserType defines the user type @@ -125,10 +126,8 @@ type User struct { UseCustomAvatar bool // Counters - NumFollowers int - NumFollowing int `xorm:"NOT NULL DEFAULT 0"` - NumStars int - NumRepos int + NumStars int + NumRepos int // For organization NumTeams int @@ -312,14 +311,25 @@ func (u *User) GenerateEmailActivateCode(email string) string { return code } -// GetUserFollowers returns range of user's followers. -func GetUserFollowers(ctx context.Context, u, viewer *User, listOptions db.ListOptions) ([]*User, int64, error) { - sess := db.GetEngine(ctx). - Select("`user`.*"). +func searchUserFollowers(ctx context.Context, u, viewer *User) *xorm.Session { + return db.GetEngine(ctx). Join("LEFT", "follow", "`user`.id=follow.user_id"). Where("follow.follow_id=?", u.ID). And("`user`.type=?", UserTypeIndividual). And(isUserVisibleToViewerCond(viewer)) +} + +func searchUserFollowing(ctx context.Context, u, viewer *User) *xorm.Session { + return db.GetEngine(ctx). + Join("LEFT", "follow", "`user`.id=follow.follow_id"). + Where("follow.user_id=?", u.ID). + And("`user`.type IN (?, ?)", UserTypeIndividual, UserTypeOrganization). + And(isUserVisibleToViewerCond(viewer)) +} + +// GetUserFollowers returns range of user's followers. +func GetUserFollowers(ctx context.Context, u, viewer *User, listOptions db.ListOptions) ([]*User, int64, error) { + sess := searchUserFollowers(ctx, u, viewer).Select("`user`.*") if listOptions.Page != 0 { sess = db.SetSessionPagination(sess, &listOptions) @@ -336,12 +346,7 @@ func GetUserFollowers(ctx context.Context, u, viewer *User, listOptions db.ListO // GetUserFollowing returns range of user's following. func GetUserFollowing(ctx context.Context, u, viewer *User, listOptions db.ListOptions) ([]*User, int64, error) { - sess := db.GetEngine(db.DefaultContext). - Select("`user`.*"). - Join("LEFT", "follow", "`user`.id=follow.follow_id"). - Where("follow.user_id=?", u.ID). - And("`user`.type IN (?, ?)", UserTypeIndividual, UserTypeOrganization). - And(isUserVisibleToViewerCond(viewer)) + sess := searchUserFollowing(ctx, u, viewer).Select("`user`.*") if listOptions.Page != 0 { sess = db.SetSessionPagination(sess, &listOptions) @@ -356,6 +361,18 @@ func GetUserFollowing(ctx context.Context, u, viewer *User, listOptions db.ListO return users, count, err } +// GetUserFollowersCount returns count of user's followers. +func GetUserFollowersCount(ctx context.Context, u, viewer *User) (int64, error) { + sess := searchUserFollowers(ctx, u, viewer).Table("`user`") + return sess.Count() +} + +// GetUserFollowingCount returns count of user's following. +func GetUserFollowingCount(ctx context.Context, u, viewer *User) (int64, error) { + sess := searchUserFollowing(ctx, u, viewer).Table("`user`") + return sess.Count() +} + // NewGitSig generates and returns the signature of given user. func (u *User) NewGitSig() *git.Signature { return &git.Signature{ diff --git a/services/convert/user.go b/services/convert/user.go index 79fcba0176f9a..6ba767bfa11d9 100644 --- a/services/convert/user.go +++ b/services/convert/user.go @@ -17,13 +17,7 @@ func ToUser(ctx context.Context, user, doer *user_model.User) *api.User { if user == nil { return nil } - authed := false - signed := false - if doer != nil { - signed = true - authed = doer.ID == user.ID || doer.IsAdmin - } - return toUser(ctx, user, signed, authed) + return toUser(ctx, user, doer, perm.AccessModeNone) } // ToUsers convert list of user_model.User to list of api.User @@ -41,12 +35,12 @@ func ToUserWithAccessMode(ctx context.Context, user *user_model.User, accessMode if user == nil { return nil } - return toUser(ctx, user, accessMode != perm.AccessModeNone, false) + return toUser(ctx, user, nil, accessMode) } // toUser convert user_model.User to api.User -// signed shall only be set if requester is logged in. authed shall only be set if user is site admin or user himself -func toUser(ctx context.Context, user *user_model.User, signed, authed bool) *api.User { +// accessMode is only used if doer is nil +func toUser(ctx context.Context, user, doer *user_model.User, accessMode perm.AccessMode) *api.User { result := &api.User{ ID: user.ID, UserName: user.Name, @@ -59,13 +53,31 @@ func toUser(ctx context.Context, user *user_model.User, signed, authed bool) *ap Website: user.Website, Description: user.Description, // counter's - Followers: user.NumFollowers, - Following: user.NumFollowing, StarredRepos: user.NumStars, } result.Visibility = user.Visibility.String() + authed := false + signed := false + if doer != nil { + signed = true + authed = doer.ID == user.ID || doer.IsAdmin + + count, err := user_model.GetUserFollowersCount(ctx, user, doer) + if err != nil { + return nil + } + result.Followers = int(count) + count, err = user_model.GetUserFollowingCount(ctx, user, doer) + if err != nil { + return nil + } + result.Following = int(count) + } else if accessMode != perm.AccessModeNone { + signed = true + } + // hide primary email if API caller is anonymous or user keep email private if signed && (!user.KeepEmailPrivate || authed) { result.Email = user.Email diff --git a/services/convert/user_test.go b/services/convert/user_test.go index 4b1effc7aa5fc..d61f3a88d10d3 100644 --- a/services/convert/user_test.go +++ b/services/convert/user_test.go @@ -7,6 +7,7 @@ import ( "testing" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" api "code.gitea.io/gitea/modules/structs" @@ -19,22 +20,22 @@ func TestUser_ToUser(t *testing.T) { user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1, IsAdmin: true}) - apiUser := toUser(db.DefaultContext, user1, true, true) + apiUser := toUser(db.DefaultContext, user1, user1, perm.AccessModeNone) assert.True(t, apiUser.IsAdmin) assert.Contains(t, apiUser.AvatarURL, "://") user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2, IsAdmin: false}) - apiUser = toUser(db.DefaultContext, user2, true, true) + apiUser = toUser(db.DefaultContext, user2, user1, perm.AccessModeNone) assert.False(t, apiUser.IsAdmin) - apiUser = toUser(db.DefaultContext, user1, false, false) + apiUser = toUser(db.DefaultContext, user1, nil, perm.AccessModeNone) assert.False(t, apiUser.IsAdmin) assert.EqualValues(t, api.VisibleTypePublic.String(), apiUser.Visibility) user31 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 31, IsAdmin: false, Visibility: api.VisibleTypePrivate}) - apiUser = toUser(db.DefaultContext, user31, true, true) + apiUser = toUser(db.DefaultContext, user31, user31, perm.AccessModeNone) assert.False(t, apiUser.IsAdmin) assert.EqualValues(t, api.VisibleTypePrivate.String(), apiUser.Visibility) } diff --git a/services/user/delete.go b/services/user/delete.go index 01e3c37b39f34..07a7eddd1ec78 100644 --- a/services/user/delete.go +++ b/services/user/delete.go @@ -52,24 +52,6 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) } // ***** END: Star ***** - // ***** START: Follow ***** - followeeIDs, err := db.FindIDs(ctx, "follow", "follow.follow_id", - builder.Eq{"follow.user_id": u.ID}) - if err != nil { - return fmt.Errorf("get all followees: %w", err) - } else if err = db.DecrByIDs(ctx, followeeIDs, "num_followers", new(user_model.User)); err != nil { - return fmt.Errorf("decrease user num_followers: %w", err) - } - - followerIDs, err := db.FindIDs(ctx, "follow", "follow.user_id", - builder.Eq{"follow.follow_id": u.ID}) - if err != nil { - return fmt.Errorf("get all followers: %w", err) - } else if err = db.DecrByIDs(ctx, followerIDs, "num_following", new(user_model.User)); err != nil { - return fmt.Errorf("decrease user num_following: %w", err) - } - // ***** END: Follow ***** - if err = db.DeleteBeans(ctx, &auth_model.AccessToken{UID: u.ID}, &repo_model.Collaboration{UserID: u.ID},