Skip to content

Commit baed01f

Browse files
lunnylunny
authored
Remove unnecessary attributes of User struct (#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
1 parent c2ab198 commit baed01f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+279
-451
lines changed

‎cmd/admin.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ func runChangePassword(c *cli.Context) error {
366366
return err
367367
}
368368

369-
if err = models.UpdateUserCols(user, "passwd", "passwd_hash_algo", "salt"); err != nil {
369+
if err = models.UpdateUserCols(db.DefaultContext, user, "passwd", "passwd_hash_algo", "salt"); err != nil {
370370
return err
371371
}
372372

‎models/access.go

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -105,66 +105,6 @@ func accessLevel(e db.Engine, user *User, repo *Repository) (AccessMode, error)
105105
return a.Mode, nil
106106
}
107107

108-
type repoAccess struct {
109-
Access `xorm:"extends"`
110-
Repository `xorm:"extends"`
111-
}
112-
113-
func (repoAccess) TableName() string {
114-
return "access"
115-
}
116-
117-
// GetRepositoryAccesses finds all repositories with their access mode where a user has access but does not own.
118-
func (user *User) GetRepositoryAccesses() (map[*Repository]AccessMode, error) {
119-
rows, err := db.GetEngine(db.DefaultContext).
120-
Join("INNER", "repository", "repository.id = access.repo_id").
121-
Where("access.user_id = ?", user.ID).
122-
And("repository.owner_id <> ?", user.ID).
123-
Rows(new(repoAccess))
124-
if err != nil {
125-
return nil, err
126-
}
127-
defer rows.Close()
128-
129-
repos := make(map[*Repository]AccessMode, 10)
130-
ownerCache := make(map[int64]*User, 10)
131-
for rows.Next() {
132-
var repo repoAccess
133-
err = rows.Scan(&repo)
134-
if err != nil {
135-
return nil, err
136-
}
137-
138-
var ok bool
139-
if repo.Owner, ok = ownerCache[repo.OwnerID]; !ok {
140-
if err = repo.GetOwner(); err != nil {
141-
return nil, err
142-
}
143-
ownerCache[repo.OwnerID] = repo.Owner
144-
}
145-
146-
repos[&repo.Repository] = repo.Access.Mode
147-
}
148-
return repos, nil
149-
}
150-
151-
// GetAccessibleRepositories finds repositories which the user has access but does not own.
152-
// If limit is smaller than 1 means returns all found results.
153-
func (user *User) GetAccessibleRepositories(limit int) (repos []*Repository, _ error) {
154-
sess := db.GetEngine(db.DefaultContext).
155-
Where("owner_id !=? ", user.ID).
156-
Desc("updated_unix")
157-
if limit > 0 {
158-
sess.Limit(limit)
159-
repos = make([]*Repository, 0, limit)
160-
} else {
161-
repos = make([]*Repository, 0, 10)
162-
}
163-
return repos, sess.
164-
Join("INNER", "access", "access.user_id = ? AND access.repo_id = repository.id", user.ID).
165-
Find(&repos)
166-
}
167-
168108
func maxAccessMode(modes ...AccessMode) AccessMode {
169109
max := AccessModeNone
170110
for _, mode := range modes {

‎models/access_test.go

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -90,39 +90,6 @@ func TestHasAccess(t *testing.T) {
9090
assert.NoError(t, err)
9191
}
9292

93-
func TestUser_GetRepositoryAccesses(t *testing.T) {
94-
assert.NoError(t, unittest.PrepareTestDatabase())
95-
96-
user1 := unittest.AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
97-
accesses, err := user1.GetRepositoryAccesses()
98-
assert.NoError(t, err)
99-
assert.Len(t, accesses, 0)
100-
101-
user29 := unittest.AssertExistsAndLoadBean(t, &User{ID: 29}).(*User)
102-
accesses, err = user29.GetRepositoryAccesses()
103-
assert.NoError(t, err)
104-
assert.Len(t, accesses, 2)
105-
}
106-
107-
func TestUser_GetAccessibleRepositories(t *testing.T) {
108-
assert.NoError(t, unittest.PrepareTestDatabase())
109-
110-
user1 := unittest.AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
111-
repos, err := user1.GetAccessibleRepositories(0)
112-
assert.NoError(t, err)
113-
assert.Len(t, repos, 0)
114-
115-
user2 := unittest.AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
116-
repos, err = user2.GetAccessibleRepositories(0)
117-
assert.NoError(t, err)
118-
assert.Len(t, repos, 4)
119-
120-
user29 := unittest.AssertExistsAndLoadBean(t, &User{ID: 29}).(*User)
121-
repos, err = user29.GetAccessibleRepositories(0)
122-
assert.NoError(t, err)
123-
assert.Len(t, repos, 2)
124-
}
125-
12693
func TestRepository_RecalculateAccesses(t *testing.T) {
12794
// test with organization repo
12895
assert.NoError(t, unittest.PrepareTestDatabase())

‎models/org.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,12 @@ func (org *Organization) HomeLink() string {
125125
return org.AsUser().HomeLink()
126126
}
127127

128+
// CanCreateRepo returns if user login can create a repository
129+
// NOTE: functions calling this assume a failure due to repository count limit; if new checks are added, those functions should be revised
130+
func (org *Organization) CanCreateRepo() bool {
131+
return org.AsUser().CanCreateRepo()
132+
}
133+
128134
// FindOrgMembersOpts represensts find org members conditions
129135
type FindOrgMembersOpts struct {
130136
db.ListOptions
@@ -240,7 +246,7 @@ func CreateOrganization(org *Organization, owner *User) (err error) {
240246
if err = db.Insert(ctx, org); err != nil {
241247
return fmt.Errorf("insert organization: %v", err)
242248
}
243-
if err = org.AsUser().generateRandomAvatar(db.GetEngine(ctx)); err != nil {
249+
if err = generateRandomAvatar(db.GetEngine(ctx), org.AsUser()); err != nil {
244250
return fmt.Errorf("generate random avatar: %v", err)
245251
}
246252

@@ -546,8 +552,8 @@ func CountOrgs(opts FindOrgOptions) (int64, error) {
546552
Count(new(User))
547553
}
548554

549-
func getOwnedOrgsByUserID(sess db.Engine, userID int64) ([]*User, error) {
550-
orgs := make([]*User, 0, 10)
555+
func getOwnedOrgsByUserID(sess db.Engine, userID int64) ([]*Organization, error) {
556+
orgs := make([]*Organization, 0, 10)
551557
return orgs, sess.
552558
Join("INNER", "`team_user`", "`team_user`.org_id=`user`.id").
553559
Join("INNER", "`team`", "`team`.id=`team_user`.team_id").
@@ -593,20 +599,20 @@ func HasOrgsVisible(orgs []*Organization, user *User) bool {
593599
}
594600

595601
// GetOwnedOrgsByUserID returns a list of organizations are owned by given user ID.
596-
func GetOwnedOrgsByUserID(userID int64) ([]*User, error) {
602+
func GetOwnedOrgsByUserID(userID int64) ([]*Organization, error) {
597603
return getOwnedOrgsByUserID(db.GetEngine(db.DefaultContext), userID)
598604
}
599605

600606
// GetOwnedOrgsByUserIDDesc returns a list of organizations are owned by
601607
// given user ID, ordered descending by the given condition.
602-
func GetOwnedOrgsByUserIDDesc(userID int64, desc string) ([]*User, error) {
608+
func GetOwnedOrgsByUserIDDesc(userID int64, desc string) ([]*Organization, error) {
603609
return getOwnedOrgsByUserID(db.GetEngine(db.DefaultContext).Desc(desc), userID)
604610
}
605611

606612
// GetOrgsCanCreateRepoByUserID returns a list of organizations where given user ID
607613
// are allowed to create repos.
608-
func GetOrgsCanCreateRepoByUserID(userID int64) ([]*User, error) {
609-
orgs := make([]*User, 0, 10)
614+
func GetOrgsCanCreateRepoByUserID(userID int64) ([]*Organization, error) {
615+
orgs := make([]*Organization, 0, 10)
610616

611617
return orgs, db.GetEngine(db.DefaultContext).Where(builder.In("id", builder.Select("`user`.id").From("`user`").
612618
Join("INNER", "`team_user`", "`team_user`.org_id = `user`.id").

‎models/repo.go

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -754,19 +754,20 @@ func (repo *Repository) UpdateSize(ctx context.Context) error {
754754
return repo.updateSize(db.GetEngine(ctx))
755755
}
756756

757-
// CanUserFork returns true if specified user can fork repository.
758-
func (repo *Repository) CanUserFork(user *User) (bool, error) {
757+
// CanUserForkRepo returns true if specified user can fork repository.
758+
func CanUserForkRepo(user *User, repo *Repository) (bool, error) {
759759
if user == nil {
760760
return false, nil
761761
}
762-
if repo.OwnerID != user.ID && !user.HasForkedRepo(repo.ID) {
762+
if repo.OwnerID != user.ID && !HasForkedRepo(user.ID, repo.ID) {
763763
return true, nil
764764
}
765-
if err := user.GetOwnedOrganizations(); err != nil {
765+
ownedOrgs, err := GetOwnedOrgsByUserID(user.ID)
766+
if err != nil {
766767
return false, err
767768
}
768-
for _, org := range user.OwnedOrgs {
769-
if repo.OwnerID != org.ID && !org.HasForkedRepo(repo.ID) {
769+
for _, org := range ownedOrgs {
770+
if repo.OwnerID != org.ID && !HasForkedRepo(org.ID, repo.ID) {
770771
return true, nil
771772
}
772773
}
@@ -2036,13 +2037,25 @@ func (repo *Repository) SetArchiveRepoState(isArchived bool) (err error) {
20362037
// \___ / \____/|__| |__|_ \
20372038
// \/ \/
20382039

2039-
// HasForkedRepo checks if given user has already forked a repository with given ID.
2040-
func HasForkedRepo(ownerID, repoID int64) (*Repository, bool) {
2040+
// GetForkedRepo checks if given user has already forked a repository with given ID.
2041+
func GetForkedRepo(ownerID, repoID int64) *Repository {
20412042
repo := new(Repository)
20422043
has, _ := db.GetEngine(db.DefaultContext).
20432044
Where("owner_id=? AND fork_id=?", ownerID, repoID).
20442045
Get(repo)
2045-
return repo, has
2046+
if has {
2047+
return repo
2048+
}
2049+
return nil
2050+
}
2051+
2052+
// HasForkedRepo checks if given user has already forked a repository with given ID.
2053+
func HasForkedRepo(ownerID, repoID int64) bool {
2054+
has, _ := db.GetEngine(db.DefaultContext).
2055+
Table("repository").
2056+
Where("owner_id=? AND fork_id=?", ownerID, repoID).
2057+
Exist()
2058+
return has
20462059
}
20472060

20482061
// CopyLFS copies LFS data from one repo to another

‎models/star.go

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func isStaring(e db.Engine, userID, repoID int64) bool {
7474
}
7575

7676
// GetStargazers returns the users that starred the repo.
77-
func (repo *Repository) GetStargazers(opts db.ListOptions) ([]*User, error) {
77+
func GetStargazers(repo *Repository, opts db.ListOptions) ([]*User, error) {
7878
sess := db.GetEngine(db.DefaultContext).Where("star.repo_id = ?", repo.ID).
7979
Join("LEFT", "star", "`user`.id = star.uid")
8080
if opts.Page > 0 {
@@ -87,48 +87,3 @@ func (repo *Repository) GetStargazers(opts db.ListOptions) ([]*User, error) {
8787
users := make([]*User, 0, 8)
8888
return users, sess.Find(&users)
8989
}
90-
91-
// GetStarredRepos returns the repos the user starred.
92-
func (u *User) GetStarredRepos(private bool, page, pageSize int, orderBy string) (repos RepositoryList, err error) {
93-
if len(orderBy) == 0 {
94-
orderBy = "updated_unix DESC"
95-
}
96-
sess := db.GetEngine(db.DefaultContext).
97-
Join("INNER", "star", "star.repo_id = repository.id").
98-
Where("star.uid = ?", u.ID).
99-
OrderBy(orderBy)
100-
101-
if !private {
102-
sess = sess.And("is_private = ?", false)
103-
}
104-
105-
if page <= 0 {
106-
page = 1
107-
}
108-
sess.Limit(pageSize, (page-1)*pageSize)
109-
110-
repos = make([]*Repository, 0, pageSize)
111-
112-
if err = sess.Find(&repos); err != nil {
113-
return
114-
}
115-
116-
if err = repos.loadAttributes(db.GetEngine(db.DefaultContext)); err != nil {
117-
return
118-
}
119-
120-
return
121-
}
122-
123-
// GetStarredRepoCount returns the numbers of repo the user starred.
124-
func (u *User) GetStarredRepoCount(private bool) (int64, error) {
125-
sess := db.GetEngine(db.DefaultContext).
126-
Join("INNER", "star", "star.repo_id = repository.id").
127-
Where("star.uid = ?", u.ID)
128-
129-
if !private {
130-
sess = sess.And("is_private = ?", false)
131-
}
132-
133-
return sess.Count(&Repository{})
134-
}

‎models/star_test.go

Lines changed: 2 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func TestRepository_GetStargazers(t *testing.T) {
3636
// repo with stargazers
3737
assert.NoError(t, unittest.PrepareTestDatabase())
3838
repo := unittest.AssertExistsAndLoadBean(t, &Repository{ID: 4}).(*Repository)
39-
gazers, err := repo.GetStargazers(db.ListOptions{Page: 0})
39+
gazers, err := GetStargazers(repo, db.ListOptions{Page: 0})
4040
assert.NoError(t, err)
4141
if assert.Len(t, gazers, 1) {
4242
assert.Equal(t, int64(2), gazers[0].ID)
@@ -47,53 +47,7 @@ func TestRepository_GetStargazers2(t *testing.T) {
4747
// repo with stargazers
4848
assert.NoError(t, unittest.PrepareTestDatabase())
4949
repo := unittest.AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
50-
gazers, err := repo.GetStargazers(db.ListOptions{Page: 0})
50+
gazers, err := GetStargazers(repo, db.ListOptions{Page: 0})
5151
assert.NoError(t, err)
5252
assert.Len(t, gazers, 0)
5353
}
54-
55-
func TestUser_GetStarredRepos(t *testing.T) {
56-
// user who has starred repos
57-
assert.NoError(t, unittest.PrepareTestDatabase())
58-
59-
user := unittest.AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
60-
starred, err := user.GetStarredRepos(false, 1, 10, "")
61-
assert.NoError(t, err)
62-
if assert.Len(t, starred, 1) {
63-
assert.Equal(t, int64(4), starred[0].ID)
64-
}
65-
66-
starred, err = user.GetStarredRepos(true, 1, 10, "")
67-
assert.NoError(t, err)
68-
if assert.Len(t, starred, 2) {
69-
assert.Equal(t, int64(2), starred[0].ID)
70-
assert.Equal(t, int64(4), starred[1].ID)
71-
}
72-
}
73-
74-
func TestUser_GetStarredRepos2(t *testing.T) {
75-
// user who has no starred repos
76-
assert.NoError(t, unittest.PrepareTestDatabase())
77-
78-
user := unittest.AssertExistsAndLoadBean(t, &User{ID: 1}).(*User)
79-
starred, err := user.GetStarredRepos(false, 1, 10, "")
80-
assert.NoError(t, err)
81-
assert.Len(t, starred, 0)
82-
83-
starred, err = user.GetStarredRepos(true, 1, 10, "")
84-
assert.NoError(t, err)
85-
assert.Len(t, starred, 0)
86-
}
87-
88-
func TestUserGetStarredRepoCount(t *testing.T) {
89-
assert.NoError(t, unittest.PrepareTestDatabase())
90-
91-
user := unittest.AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
92-
counts, err := user.GetStarredRepoCount(false)
93-
assert.NoError(t, err)
94-
assert.Equal(t, int64(1), counts)
95-
96-
counts, err = user.GetStarredRepoCount(true)
97-
assert.NoError(t, err)
98-
assert.Equal(t, int64(2), counts)
99-
}

0 commit comments

Comments
 (0)