From 9ae61bbe661b1b4d6894813b46cc9c2df399600f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 12 Nov 2024 13:05:00 -0800 Subject: [PATCH 1/6] Reimplement GetUserOrgsList to make it simple and clear --- models/organization/mini_org.go | 78 -------------- models/organization/org.go | 57 ---------- models/organization/org_list.go | 130 +++++++++++++++++++++++ models/organization/org_list_test.go | 61 +++++++++++ models/organization/org_test.go | 36 ------- services/oauth2_provider/access_token.go | 6 +- 6 files changed, 196 insertions(+), 172 deletions(-) delete mode 100644 models/organization/mini_org.go create mode 100644 models/organization/org_list.go create mode 100644 models/organization/org_list_test.go diff --git a/models/organization/mini_org.go b/models/organization/mini_org.go deleted file mode 100644 index b1b24624c5a21..0000000000000 --- a/models/organization/mini_org.go +++ /dev/null @@ -1,78 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package organization - -import ( - "context" - "fmt" - "strings" - - "code.gitea.io/gitea/models/db" - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" - user_model "code.gitea.io/gitea/models/user" - - "xorm.io/builder" -) - -// MinimalOrg represents a simple organization with only the needed columns -type MinimalOrg = Organization - -// GetUserOrgsList returns all organizations the given user has access to -func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg, error) { - schema, err := db.TableInfo(new(user_model.User)) - if err != nil { - return nil, err - } - - outputCols := []string{ - "id", - "name", - "full_name", - "visibility", - "avatar", - "avatar_email", - "use_custom_avatar", - } - - groupByCols := &strings.Builder{} - for _, col := range outputCols { - fmt.Fprintf(groupByCols, "`%s`.%s,", schema.Name, col) - } - groupByStr := groupByCols.String() - groupByStr = groupByStr[0 : len(groupByStr)-1] - - sess := db.GetEngine(ctx) - sess = sess.Select(groupByStr+", count(distinct repo_id) as org_count"). - Table("user"). - Join("INNER", "team", "`team`.org_id = `user`.id"). - Join("INNER", "team_user", "`team`.id = `team_user`.team_id"). - Join("LEFT", builder. - Select("id as repo_id, owner_id as repo_owner_id"). - From("repository"). - Where(repo_model.AccessibleRepositoryCondition(user, unit.TypeInvalid)), "`repository`.repo_owner_id = `team`.org_id"). - Where("`team_user`.uid = ?", user.ID). - GroupBy(groupByStr) - - type OrgCount struct { - Organization `xorm:"extends"` - OrgCount int - } - - orgCounts := make([]*OrgCount, 0, 10) - - if err := sess. - Asc("`user`.name"). - Find(&orgCounts); err != nil { - return nil, err - } - - orgs := make([]*MinimalOrg, len(orgCounts)) - for i, orgCount := range orgCounts { - orgCount.Organization.NumRepos = orgCount.OrgCount - orgs[i] = &orgCount.Organization - } - - return orgs, nil -} diff --git a/models/organization/org.go b/models/organization/org.go index 6231f1eeedf58..725a99356e974 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -25,13 +25,6 @@ import ( "xorm.io/xorm" ) -// ________ .__ __ .__ -// \_____ \_______ _________ ____ |__|____________ _/ |_|__| ____ ____ -// / | \_ __ \/ ___\__ \ / \| \___ /\__ \\ __\ |/ _ \ / \ -// / | \ | \/ /_/ > __ \| | \ |/ / / __ \| | | ( <_> ) | \ -// \_______ /__| \___ (____ /___| /__/_____ \(____ /__| |__|\____/|___| / -// \/ /_____/ \/ \/ \/ \/ \/ - // ErrOrgNotExist represents a "OrgNotExist" kind of error. type ErrOrgNotExist struct { ID int64 @@ -465,42 +458,6 @@ func GetUsersWhoCanCreateOrgRepo(ctx context.Context, orgID int64) (map[int64]*u And("team_user.org_id = ?", orgID).Find(&users) } -// SearchOrganizationsOptions options to filter organizations -type SearchOrganizationsOptions struct { - db.ListOptions - All bool -} - -// FindOrgOptions finds orgs options -type FindOrgOptions struct { - db.ListOptions - UserID int64 - IncludePrivate bool -} - -func queryUserOrgIDs(userID int64, includePrivate bool) *builder.Builder { - cond := builder.Eq{"uid": userID} - if !includePrivate { - cond["is_public"] = true - } - return builder.Select("org_id").From("org_user").Where(cond) -} - -func (opts FindOrgOptions) ToConds() builder.Cond { - var cond builder.Cond = builder.Eq{"`user`.`type`": user_model.UserTypeOrganization} - if opts.UserID > 0 { - cond = cond.And(builder.In("`user`.`id`", queryUserOrgIDs(opts.UserID, opts.IncludePrivate))) - } - if !opts.IncludePrivate { - cond = cond.And(builder.Eq{"`user`.visibility": structs.VisibleTypePublic}) - } - return cond -} - -func (opts FindOrgOptions) ToOrders() string { - return "`user`.name ASC" -} - // HasOrgOrUserVisible tells if the given user can see the given org or user func HasOrgOrUserVisible(ctx context.Context, orgOrUser, user *user_model.User) bool { // If user is nil, it's an anonymous user/request. @@ -533,20 +490,6 @@ func HasOrgsVisible(ctx context.Context, orgs []*Organization, user *user_model. return false } -// GetOrgsCanCreateRepoByUserID returns a list of organizations where given user ID -// are allowed to create repos. -func GetOrgsCanCreateRepoByUserID(ctx context.Context, userID int64) ([]*Organization, error) { - orgs := make([]*Organization, 0, 10) - - return orgs, db.GetEngine(ctx).Where(builder.In("id", builder.Select("`user`.id").From("`user`"). - Join("INNER", "`team_user`", "`team_user`.org_id = `user`.id"). - Join("INNER", "`team`", "`team`.id = `team_user`.team_id"). - Where(builder.Eq{"`team_user`.uid": userID}). - And(builder.Eq{"`team`.authorize": perm.AccessModeOwner}.Or(builder.Eq{"`team`.can_create_org_repo": true})))). - Asc("`user`.name"). - Find(&orgs) -} - // GetOrgUsersByOrgID returns all organization-user relations by organization ID. func GetOrgUsersByOrgID(ctx context.Context, opts *FindOrgMembersOpts) ([]*OrgUser, error) { sess := db.GetEngine(ctx).Where("org_id=?", opts.OrgID) diff --git a/models/organization/org_list.go b/models/organization/org_list.go new file mode 100644 index 0000000000000..388daf9e23eb8 --- /dev/null +++ b/models/organization/org_list.go @@ -0,0 +1,130 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package organization + +import ( + "context" + "fmt" + "strings" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/perm" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/structs" + + "xorm.io/builder" +) + +// SearchOrganizationsOptions options to filter organizations +type SearchOrganizationsOptions struct { + db.ListOptions + All bool +} + +// FindOrgOptions finds orgs options +type FindOrgOptions struct { + db.ListOptions + UserID int64 + IncludePrivate bool +} + +func queryUserOrgIDs(userID int64, includePrivate bool) *builder.Builder { + cond := builder.Eq{"uid": userID} + if !includePrivate { + cond["is_public"] = true + } + return builder.Select("org_id").From("org_user").Where(cond) +} + +func (opts FindOrgOptions) ToConds() builder.Cond { + var cond builder.Cond = builder.Eq{"`user`.`type`": user_model.UserTypeOrganization} + if opts.UserID > 0 { + cond = cond.And(builder.In("`user`.`id`", queryUserOrgIDs(opts.UserID, opts.IncludePrivate))) + } + if !opts.IncludePrivate { + cond = cond.And(builder.Eq{"`user`.visibility": structs.VisibleTypePublic}) + } + return cond +} + +func (opts FindOrgOptions) ToOrders() string { + return "`user`.name ASC" +} + +// GetOrgsCanCreateRepoByUserID returns a list of organizations where given user ID +// are allowed to create repos. +func GetOrgsCanCreateRepoByUserID(ctx context.Context, userID int64) ([]*Organization, error) { + orgs := make([]*Organization, 0, 10) + + return orgs, db.GetEngine(ctx).Where(builder.In("id", builder.Select("`user`.id").From("`user`"). + Join("INNER", "`team_user`", "`team_user`.org_id = `user`.id"). + Join("INNER", "`team`", "`team`.id = `team_user`.team_id"). + Where(builder.Eq{"`team_user`.uid": userID}). + And(builder.Eq{"`team`.authorize": perm.AccessModeOwner}.Or(builder.Eq{"`team`.can_create_org_repo": true})))). + Asc("`user`.name"). + Find(&orgs) +} + +// MinimalOrg represents a simple organization with only the needed columns +type MinimalOrg = Organization + +// GetUserOrgsList returns all organizations the given user has access to +func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg, error) { + schema, err := db.TableInfo(new(user_model.User)) + if err != nil { + return nil, err + } + + outputCols := []string{ + "id", + "name", + "full_name", + "visibility", + "avatar", + "avatar_email", + "use_custom_avatar", + } + + selectColumns := &strings.Builder{} + for i, col := range outputCols { + fmt.Fprintf(selectColumns, "`%s`.%s", schema.Name, col) + if i < len(outputCols)-1 { + selectColumns.WriteString(", ") + } + } + columnsStr := selectColumns.String() + + var orgs []*MinimalOrg + if err := db.GetEngine(ctx).Select(columnsStr). + Table("user"). + Where(builder.In("`user`.`id`", queryUserOrgIDs(user.ID, true))). + Find(&orgs); err != nil { + return nil, err + } + + type orgCount struct { + OrgID int64 + RepoCount int + } + var orgCounts []orgCount + if err := db.GetEngine(ctx). + Select("team.org_id, COUNT(DISTINCT(team_repo.repo_id)) as repo_count"). + Table("team"). + Join("INNER", "team_repo", "team_repo.team_id = team.id"). + Where(builder.In("`team`.`org_id`", queryUserOrgIDs(user.ID, true))). + GroupBy("team.org_id").Find(&orgCounts); err != nil { + return nil, err + } + + orgCountMap := make(map[int64]int) + for _, orgCount := range orgCounts { + orgCountMap[orgCount.OrgID] = orgCount.RepoCount + } + + for _, org := range orgs { + org.NumRepos = orgCountMap[org.ID] + } + + return orgs, nil +} diff --git a/models/organization/org_list_test.go b/models/organization/org_list_test.go new file mode 100644 index 0000000000000..8b91f0d2cc58c --- /dev/null +++ b/models/organization/org_list_test.go @@ -0,0 +1,61 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package organization_test + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" +) + +func TestCountOrganizations(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + expected, err := db.GetEngine(db.DefaultContext).Where("type=?", user_model.UserTypeOrganization).Count(&organization.Organization{}) + assert.NoError(t, err) + cnt, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{IncludePrivate: true}) + assert.NoError(t, err) + assert.Equal(t, expected, cnt) +} + +func TestFindOrgs(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + orgs, err := db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ + UserID: 4, + IncludePrivate: true, + }) + assert.NoError(t, err) + if assert.Len(t, orgs, 1) { + assert.EqualValues(t, 3, orgs[0].ID) + } + + orgs, err = db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ + UserID: 4, + IncludePrivate: false, + }) + assert.NoError(t, err) + assert.Len(t, orgs, 0) + + total, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ + UserID: 4, + IncludePrivate: true, + }) + assert.NoError(t, err) + assert.EqualValues(t, 1, total) +} + +func TestGetUserOrgsList(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + orgs, err := organization.GetUserOrgsList(db.DefaultContext, &user_model.User{ID: 4}) + assert.NoError(t, err) + if assert.Len(t, orgs, 1) { + assert.EqualValues(t, 3, orgs[0].ID) + assert.EqualValues(t, 3, orgs[0].NumRepos) + } +} diff --git a/models/organization/org_test.go b/models/organization/org_test.go index c614aaacf56e5..7159f0fc46510 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -129,15 +129,6 @@ func TestGetOrgByName(t *testing.T) { assert.True(t, organization.IsErrOrgNotExist(err)) } -func TestCountOrganizations(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - expected, err := db.GetEngine(db.DefaultContext).Where("type=?", user_model.UserTypeOrganization).Count(&organization.Organization{}) - assert.NoError(t, err) - cnt, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{IncludePrivate: true}) - assert.NoError(t, err) - assert.Equal(t, expected, cnt) -} - func TestIsOrganizationOwner(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) test := func(orgID, userID int64, expected bool) { @@ -251,33 +242,6 @@ func TestRestrictedUserOrgMembers(t *testing.T) { } } -func TestFindOrgs(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - orgs, err := db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ - UserID: 4, - IncludePrivate: true, - }) - assert.NoError(t, err) - if assert.Len(t, orgs, 1) { - assert.EqualValues(t, 3, orgs[0].ID) - } - - orgs, err = db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ - UserID: 4, - IncludePrivate: false, - }) - assert.NoError(t, err) - assert.Len(t, orgs, 0) - - total, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ - UserID: 4, - IncludePrivate: true, - }) - assert.NoError(t, err) - assert.EqualValues(t, 1, total) -} - func TestGetOrgUsersByOrgID(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/services/oauth2_provider/access_token.go b/services/oauth2_provider/access_token.go index 00c960caf2ee7..f79afa4b301ba 100644 --- a/services/oauth2_provider/access_token.go +++ b/services/oauth2_provider/access_token.go @@ -8,6 +8,7 @@ import ( "fmt" auth "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" org_model "code.gitea.io/gitea/models/organization" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" @@ -192,7 +193,10 @@ func NewAccessTokenResponse(ctx context.Context, grant *auth.OAuth2Grant, server // returns a list of "org" and "org:team" strings, // that the given user is a part of. func GetOAuthGroupsForUser(ctx context.Context, user *user_model.User) ([]string, error) { - orgs, err := org_model.GetUserOrgsList(ctx, user) + orgs, err := db.Find[org_model.Organization](ctx, org_model.FindOrgOptions{ + UserID: user.ID, + IncludePrivate: true, + }) if err != nil { return nil, fmt.Errorf("GetUserOrgList: %w", err) } From 0cf938cc65d0a9356a67ac7ac47725f33dc4940b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 12 Nov 2024 13:57:17 -0800 Subject: [PATCH 2/6] Fix bug --- models/organization/org_list.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/models/organization/org_list.go b/models/organization/org_list.go index 388daf9e23eb8..8ab311ee81f35 100644 --- a/models/organization/org_list.go +++ b/models/organization/org_list.go @@ -112,7 +112,10 @@ func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg, Select("team.org_id, COUNT(DISTINCT(team_repo.repo_id)) as repo_count"). Table("team"). Join("INNER", "team_repo", "team_repo.team_id = team.id"). - Where(builder.In("`team`.`org_id`", queryUserOrgIDs(user.ID, true))). + Where(builder.In("`team`.`id`", + builder.Select("team_id").From("team_user"). + Where(builder.Eq{"uid": user.ID}), + )). GroupBy("team.org_id").Find(&orgCounts); err != nil { return nil, err } From 1ed9934c1104c28e03bf95d99aed3e38ee6f4072 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 12 Nov 2024 13:58:13 -0800 Subject: [PATCH 3/6] Add comment about the repository counting --- models/organization/org_list.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/organization/org_list.go b/models/organization/org_list.go index 8ab311ee81f35..8c745c155ad9c 100644 --- a/models/organization/org_list.go +++ b/models/organization/org_list.go @@ -107,6 +107,7 @@ func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg, OrgID int64 RepoCount int } + // FIXME: This doesn't counting those public repos in the organization that the user has access to var orgCounts []orgCount if err := db.GetEngine(ctx). Select("team.org_id, COUNT(DISTINCT(team_repo.repo_id)) as repo_count"). From 1a152ad69ff5d523cf19dd9b1ae2a46e5774b828 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 12 Nov 2024 19:28:39 -0800 Subject: [PATCH 4/6] Fix bug --- models/organization/org_list.go | 20 ++++++++++++-------- models/organization/org_list_test.go | 3 ++- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/models/organization/org_list.go b/models/organization/org_list.go index 8c745c155ad9c..b3ee36c40e96d 100644 --- a/models/organization/org_list.go +++ b/models/organization/org_list.go @@ -107,17 +107,21 @@ func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg, OrgID int64 RepoCount int } - // FIXME: This doesn't counting those public repos in the organization that the user has access to var orgCounts []orgCount if err := db.GetEngine(ctx). - Select("team.org_id, COUNT(DISTINCT(team_repo.repo_id)) as repo_count"). - Table("team"). - Join("INNER", "team_repo", "team_repo.team_id = team.id"). - Where(builder.In("`team`.`id`", - builder.Select("team_id").From("team_user"). - Where(builder.Eq{"uid": user.ID}), + Select("owner_id AS org_id, COUNT(DISTINCT(repository.id)) as repo_count"). + Table("repository"). + Join("INNER", "org_user", "owner_id = org_user.org_id"). + Where("org_user.uid = ?", user.ID). + And(builder.Or( + builder.Eq{"repository.is_private": false}, + builder.In("repository.id", builder.Select("repo_id").From("team_repo"). + InnerJoin("team_user", "team_user.team_id = team_repo.team_id"). + Where(builder.Eq{"team_user.uid": user.ID})), + builder.In("repository.id", builder.Select("repo_id").From("collaboration"). + Where(builder.Eq{"user_id": user.ID})), )). - GroupBy("team.org_id").Find(&orgCounts); err != nil { + GroupBy("owner_id").Find(&orgCounts); err != nil { return nil, err } diff --git a/models/organization/org_list_test.go b/models/organization/org_list_test.go index 8b91f0d2cc58c..fc8d148a1d7c0 100644 --- a/models/organization/org_list_test.go +++ b/models/organization/org_list_test.go @@ -56,6 +56,7 @@ func TestGetUserOrgsList(t *testing.T) { assert.NoError(t, err) if assert.Len(t, orgs, 1) { assert.EqualValues(t, 3, orgs[0].ID) - assert.EqualValues(t, 3, orgs[0].NumRepos) + // repo_id: 3 is in the team, 32 is public, 5 is private with no team + assert.EqualValues(t, 2, orgs[0].NumRepos) } } From 068851795865368faf16804d2dda092aeccc1c4e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Nov 2024 11:47:21 +0800 Subject: [PATCH 5/6] Update models/organization/org_list.go Co-authored-by: Zettat123 --- models/organization/org_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/organization/org_list.go b/models/organization/org_list.go index b3ee36c40e96d..a6798ed7b732b 100644 --- a/models/organization/org_list.go +++ b/models/organization/org_list.go @@ -125,7 +125,7 @@ func GetUserOrgsList(ctx context.Context, user *user_model.User) ([]*MinimalOrg, return nil, err } - orgCountMap := make(map[int64]int) + orgCountMap := make(map[int64]int, len(orgCounts)) for _, orgCount := range orgCounts { orgCountMap[orgCount.OrgID] = orgCount.RepoCount } From 877efb625d2bf2ce4b3be9090416da75737e87ac Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 13 Nov 2024 20:00:09 -0800 Subject: [PATCH 6/6] Fix order --- models/organization/org_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/organization/org_list.go b/models/organization/org_list.go index b3ee36c40e96d..dc7358d7e9d73 100644 --- a/models/organization/org_list.go +++ b/models/organization/org_list.go @@ -49,7 +49,7 @@ func (opts FindOrgOptions) ToConds() builder.Cond { } func (opts FindOrgOptions) ToOrders() string { - return "`user`.name ASC" + return "`user`.lower_name ASC" } // GetOrgsCanCreateRepoByUserID returns a list of organizations where given user ID