Remove unnecessary attributes of User struct (go-gitea#17745)

* Remove unnecessary functions of User struct

* Move more database methods out of user struct

* Move more database methods out of user struct

* Fix template failure

* Fix bug

* Remove finished FIXME

* remove unnecessary code

cmd/admin.go

@@ -366,7 +366,7 @@ func runChangePassword(c *cli.Context) error {
 		return err
 	}
 
-	if err = models.UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil {
+	if err = models.UpdateUserCols(db.DefaultContext, user, "passwd", "passwd_hash_algo", "salt"); err != nil {
 		return err
 	}
 

models/access.go

@@ -105,66 +105,6 @@ func accessLevel(e db.Engine, user *User, repo *Repository) (AccessMode, error)
 	return a.Mode, nil
 }
 
-type repoAccess struct {
-	Access     `xorm:"extends"`
-	Repository `xorm:"extends"`
-}
-
-func (repoAccess) TableName() string {
-	return "access"
-}
-
-// GetRepositoryAccesses finds all repositories with their access mode where a user has access but does not own.
-func (user *User) GetRepositoryAccesses() (map[*Repository]AccessMode, error) {
-	rows, err := db.GetEngine(db.DefaultContext).
-		Join("INNER", "repository", "repository.id = access.repo_id").
-		Where("access.user_id = ?", user.ID).
-		And("repository.owner_id <> ?", user.ID).
-		Rows(new(repoAccess))
-	if err != nil {
-		return nil, err
-	}
-	defer rows.Close()
-
-	repos := make(map[*Repository]AccessMode, 10)
-	ownerCache := make(map[int64]*User, 10)
-	for rows.Next() {
-		var repo repoAccess
-		err = rows.Scan(&repo)
-		if err != nil {
-			return nil, err
-		}
-
-		var ok bool
-		if repo.Owner, ok = ownerCache[repo.OwnerID]; !ok {
-			if err = repo.GetOwner(); err != nil {
-				return nil, err
-			}
-			ownerCache[repo.OwnerID] = repo.Owner
-		}
-
-		repos[&repo.Repository] = repo.Access.Mode
-	}
-	return repos, nil
-}
-
-// GetAccessibleRepositories finds repositories which the user has access but does not own.
-// If limit is smaller than 1 means returns all found results.
-func (user *User) GetAccessibleRepositories(limit int) (repos []*Repository, _ error) {
-	sess := db.GetEngine(db.DefaultContext).
-		Where("owner_id !=? ", user.ID).
-		Desc("updated_unix")
-	if limit > 0 {
-		sess.Limit(limit)
-		repos = make([]*Repository, 0, limit)
-	} else {
-		repos = make([]*Repository, 0, 10)
-	}
-	return repos, sess.
-		Join("INNER", "access", "access.user_id = ? AND access.repo_id = repository.id", user.ID).
-		Find(&repos)
-}
-
 func maxAccessMode(modes ...AccessMode) AccessMode {
 	max := AccessModeNone
 	for _, mode := range modes {

models/access_test.go

@@ -90,39 +90,6 @@ func TestHasAccess(t *testing.T) {
 	assert.NoError(t, err)
 }
 
-func TestUser_GetRepositoryAccesses(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	user1 := unittest.AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
-	accesses, err := user1.GetRepositoryAccesses()
-	assert.NoError(t, err)
-	assert.Len(t, accesses, 0)
-
-	user29 := unittest.AssertExistsAndLoadBean(t, &User{ID: 29}).(*User)
-	accesses, err = user29.GetRepositoryAccesses()
-	assert.NoError(t, err)
-	assert.Len(t, accesses, 2)
-}
-
-func TestUser_GetAccessibleRepositories(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	user1 := unittest.AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
-	repos, err := user1.GetAccessibleRepositories(0)
-	assert.NoError(t, err)
-	assert.Len(t, repos, 0)
-
-	user2 := unittest.AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
-	repos, err = user2.GetAccessibleRepositories(0)
-	assert.NoError(t, err)
-	assert.Len(t, repos, 4)
-
-	user29 := unittest.AssertExistsAndLoadBean(t, &User{ID: 29}).(*User)
-	repos, err = user29.GetAccessibleRepositories(0)
-	assert.NoError(t, err)
-	assert.Len(t, repos, 2)
-}
-
 func TestRepository_RecalculateAccesses(t *testing.T) {
 	// test with organization repo
 	assert.NoError(t, unittest.PrepareTestDatabase())

models/org.go

@@ -125,6 +125,12 @@ func (org *Organization) HomeLink() string {
 	return org.AsUser().HomeLink()
 }
 
+// CanCreateRepo returns if user login can create a repository
+// NOTE: functions calling this assume a failure due to repository count limit; if new checks are added, those functions should be revised
+func (org *Organization) CanCreateRepo() bool {
+	return org.AsUser().CanCreateRepo()
+}
+
 // FindOrgMembersOpts represensts find org members conditions
 type FindOrgMembersOpts struct {
 	db.ListOptions
@@ -240,7 +246,7 @@ func CreateOrganization(org *Organization, owner *User) (err error) {
 	if err = db.Insert(ctx, org); err != nil {
 		return fmt.Errorf("insert organization: %v", err)
 	}
-	if err = org.AsUser().generateRandomAvatar(db.GetEngine(ctx)); err != nil {
+	if err = generateRandomAvatar(db.GetEngine(ctx), org.AsUser()); err != nil {
 		return fmt.Errorf("generate random avatar: %v", err)
 	}
 
@@ -546,8 +552,8 @@ func CountOrgs(opts FindOrgOptions) (int64, error) {
 		Count(new(User))
 }
 
-func getOwnedOrgsByUserID(sess db.Engine, userID int64) ([]*User, error) {
-	orgs := make([]*User, 0, 10)
+func getOwnedOrgsByUserID(sess db.Engine, userID int64) ([]*Organization, error) {
+	orgs := make([]*Organization, 0, 10)
 	return orgs, sess.
 		Join("INNER", "`team_user`", "`team_user`.org_id=`user`.id").
 		Join("INNER", "`team`", "`team`.id=`team_user`.team_id").
@@ -593,20 +599,20 @@ func HasOrgsVisible(orgs []*Organization, user *User) bool {
 }
 
 // GetOwnedOrgsByUserID returns a list of organizations are owned by given user ID.
-func GetOwnedOrgsByUserID(userID int64) ([]*User, error) {
+func GetOwnedOrgsByUserID(userID int64) ([]*Organization, error) {
 	return getOwnedOrgsByUserID(db.GetEngine(db.DefaultContext), userID)
 }
 
 // GetOwnedOrgsByUserIDDesc returns a list of organizations are owned by
 // given user ID, ordered descending by the given condition.
-func GetOwnedOrgsByUserIDDesc(userID int64, desc string) ([]*User, error) {
+func GetOwnedOrgsByUserIDDesc(userID int64, desc string) ([]*Organization, error) {
 	return getOwnedOrgsByUserID(db.GetEngine(db.DefaultContext).Desc(desc), userID)
 }
 
 // GetOrgsCanCreateRepoByUserID returns a list of organizations where given user ID
 // are allowed to create repos.
-func GetOrgsCanCreateRepoByUserID(userID int64) ([]*User, error) {
-	orgs := make([]*User, 0, 10)
+func GetOrgsCanCreateRepoByUserID(userID int64) ([]*Organization, error) {
+	orgs := make([]*Organization, 0, 10)
 
 	return orgs, db.GetEngine(db.DefaultContext).Where(builder.In("id", builder.Select("`user`.id").From("`user`").
 		Join("INNER", "`team_user`", "`team_user`.org_id = `user`.id").

models/repo.go

@@ -754,19 +754,20 @@ func (repo *Repository) UpdateSize(ctx context.Context) error {
 	return repo.updateSize(db.GetEngine(ctx))
 }
 
-// CanUserFork returns true if specified user can fork repository.
-func (repo *Repository) CanUserFork(user *User) (bool, error) {
+// CanUserForkRepo returns true if specified user can fork repository.
+func CanUserForkRepo(user *User, repo *Repository) (bool, error) {
 	if user == nil {
 		return false, nil
 	}
-	if repo.OwnerID != user.ID && !user.HasForkedRepo(repo.ID) {
+	if repo.OwnerID != user.ID && !HasForkedRepo(user.ID, repo.ID) {
 		return true, nil
 	}
-	if err := user.GetOwnedOrganizations(); err != nil {
+	ownedOrgs, err := GetOwnedOrgsByUserID(user.ID)
+	if err != nil {
 		return false, err
 	}
-	for _, org := range user.OwnedOrgs {
-		if repo.OwnerID != org.ID && !org.HasForkedRepo(repo.ID) {
+	for _, org := range ownedOrgs {
+		if repo.OwnerID != org.ID && !HasForkedRepo(org.ID, repo.ID) {
 			return true, nil
 		}
 	}
@@ -2036,13 +2037,25 @@ func (repo *Repository) SetArchiveRepoState(isArchived bool) (err error) {
 //  \___  / \____/|__|  |__|_ \
 //      \/                   \/
 
-// HasForkedRepo checks if given user has already forked a repository with given ID.
-func HasForkedRepo(ownerID, repoID int64) (*Repository, bool) {
+// GetForkedRepo checks if given user has already forked a repository with given ID.
+func GetForkedRepo(ownerID, repoID int64) *Repository {
 	repo := new(Repository)
 	has, _ := db.GetEngine(db.DefaultContext).
 		Where("owner_id=? AND fork_id=?", ownerID, repoID).
 		Get(repo)
-	return repo, has
+	if has {
+		return repo
+	}
+	return nil
+}
+
+// HasForkedRepo checks if given user has already forked a repository with given ID.
+func HasForkedRepo(ownerID, repoID int64) bool {
+	has, _ := db.GetEngine(db.DefaultContext).
+		Table("repository").
+		Where("owner_id=? AND fork_id=?", ownerID, repoID).
+		Exist()
+	return has
 }
 
 // CopyLFS copies LFS data from one repo to another

models/star.go

@@ -74,7 +74,7 @@ func isStaring(e db.Engine, userID, repoID int64) bool {
 }
 
 // GetStargazers returns the users that starred the repo.
-func (repo *Repository) GetStargazers(opts db.ListOptions) ([]*User, error) {
+func GetStargazers(repo *Repository, opts db.ListOptions) ([]*User, error) {
 	sess := db.GetEngine(db.DefaultContext).Where("star.repo_id = ?", repo.ID).
 		Join("LEFT", "star", "`user`.id = star.uid")
 	if opts.Page > 0 {
@@ -87,48 +87,3 @@ func (repo *Repository) GetStargazers(opts db.ListOptions) ([]*User, error) {
 	users := make([]*User, 0, 8)
 	return users, sess.Find(&users)
 }
-
-// GetStarredRepos returns the repos the user starred.
-func (u *User) GetStarredRepos(private bool, page, pageSize int, orderBy string) (repos RepositoryList, err error) {
-	if len(orderBy) == 0 {
-		orderBy = "updated_unix DESC"
-	}
-	sess := db.GetEngine(db.DefaultContext).
-		Join("INNER", "star", "star.repo_id = repository.id").
-		Where("star.uid = ?", u.ID).
-		OrderBy(orderBy)
-
-	if !private {
-		sess = sess.And("is_private = ?", false)
-	}
-
-	if page <= 0 {
-		page = 1
-	}
-	sess.Limit(pageSize, (page-1)*pageSize)
-
-	repos = make([]*Repository, 0, pageSize)
-
-	if err = sess.Find(&repos); err != nil {
-		return
-	}
-
-	if err = repos.loadAttributes(db.GetEngine(db.DefaultContext)); err != nil {
-		return
-	}
-
-	return
-}
-
-// GetStarredRepoCount returns the numbers of repo the user starred.
-func (u *User) GetStarredRepoCount(private bool) (int64, error) {
-	sess := db.GetEngine(db.DefaultContext).
-		Join("INNER", "star", "star.repo_id = repository.id").
-		Where("star.uid = ?", u.ID)
-
-	if !private {
-		sess = sess.And("is_private = ?", false)
-	}
-
-	return sess.Count(&Repository{})
-}

models/star_test.go

@@ -36,7 +36,7 @@ func TestRepository_GetStargazers(t *testing.T) {
 	// repo with stargazers
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	repo := unittest.AssertExistsAndLoadBean(t, &Repository{ID: 4}).(*Repository)
-	gazers, err := repo.GetStargazers(db.ListOptions{Page: 0})
+	gazers, err := GetStargazers(repo, db.ListOptions{Page: 0})
 	assert.NoError(t, err)
 	if assert.Len(t, gazers, 1) {
 		assert.Equal(t, int64(2), gazers[0].ID)
@@ -47,53 +47,7 @@ func TestRepository_GetStargazers2(t *testing.T) {
 	// repo with stargazers
 	assert.NoError(t, unittest.PrepareTestDatabase())
 	repo := unittest.AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
-	gazers, err := repo.GetStargazers(db.ListOptions{Page: 0})
+	gazers, err := GetStargazers(repo, db.ListOptions{Page: 0})
 	assert.NoError(t, err)
 	assert.Len(t, gazers, 0)
 }
-
-func TestUser_GetStarredRepos(t *testing.T) {
-	// user who has starred repos
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	user := unittest.AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
-	starred, err := user.GetStarredRepos(false, 1, 10, "")
-	assert.NoError(t, err)
-	if assert.Len(t, starred, 1) {
-		assert.Equal(t, int64(4), starred[0].ID)
-	}
-
-	starred, err = user.GetStarredRepos(true, 1, 10, "")
-	assert.NoError(t, err)
-	if assert.Len(t, starred, 2) {
-		assert.Equal(t, int64(2), starred[0].ID)
-		assert.Equal(t, int64(4), starred[1].ID)
-	}
-}
-
-func TestUser_GetStarredRepos2(t *testing.T) {
-	// user who has no starred repos
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	user := unittest.AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
-	starred, err := user.GetStarredRepos(false, 1, 10, "")
-	assert.NoError(t, err)
-	assert.Len(t, starred, 0)
-
-	starred, err = user.GetStarredRepos(true, 1, 10, "")
-	assert.NoError(t, err)
-	assert.Len(t, starred, 0)
-}
-
-func TestUserGetStarredRepoCount(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	user := unittest.AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
-	counts, err := user.GetStarredRepoCount(false)
-	assert.NoError(t, err)
-	assert.Equal(t, int64(1), counts)
-
-	counts, err = user.GetStarredRepoCount(true)
-	assert.NoError(t, err)
-	assert.Equal(t, int64(2), counts)
-}