From b4abb6deff14b741c7666d7579e0eea68443306c Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Wed, 13 Nov 2024 21:31:47 -0800
Subject: [PATCH] Reimplement GetUserOrgsList to make it simple and clear
(#32486)
Reimplement GetUserOrgsList and also move some functions and test to
org_list file.
---------
Co-authored-by: Zettat123 <zettat123@gmail.com>
---
models/organization/mini_org.go | 78 -------------
models/organization/org.go | 57 ----------
models/organization/org_list.go | 138 +++++++++++++++++++++++
models/organization/org_list_test.go | 62 ++++++++++
models/organization/org_test.go | 36 ------
services/oauth2_provider/access_token.go | 6 +-
6 files changed, 205 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 b1b24624c5..0000000000
--- 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 6231f1eeed..725a99356e 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 0000000000..72ebf6f178
--- /dev/null
+++ b/models/organization/org_list.go
@@ -0,0 +1,138 @@
+// 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`.lower_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("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("owner_id").Find(&orgCounts); err != nil {
+ return nil, err
+ }
+
+ orgCountMap := make(map[int64]int, len(orgCounts))
+ 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 0000000000..fc8d148a1d
--- /dev/null
+++ b/models/organization/org_list_test.go
@@ -0,0 +1,62 @@
+// 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)
+ // repo_id: 3 is in the team, 32 is public, 5 is private with no team
+ assert.EqualValues(t, 2, orgs[0].NumRepos)
+ }
+}
diff --git a/models/organization/org_test.go b/models/organization/org_test.go
index c614aaacf5..7159f0fc46 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 00c960caf2..f79afa4b30 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)
}