refactor(db): migrate methods off `user.go` and `org.go` (#7219) (#7227)

internal/db/db.go

@@ -124,6 +124,7 @@ func Init(w logger.Writer) (*gorm.DB, error) {
 	Follows = NewFollowsStore(db)
 	LoginSources = &loginSources{DB: db, files: sourceFiles}
 	LFS = &lfs{DB: db}
+	Orgs = NewOrgsStore(db)
 	OrgUsers = NewOrgUsersStore(db)
 	Perms = &perms{DB: db}
 	Repos = NewReposStore(db)

internal/db/org.go

@@ -297,17 +297,6 @@ func GetOwnedOrgsByUserIDDesc(userID int64, desc string) ([]*User, error) {
 	return getOwnedOrgsByUserID(sess.Desc(desc), userID)
 }
 
-// GetOrgIDsByUserID returns a list of organization IDs that user belongs to.
-// The showPrivate indicates whether to include private memberships.
-func GetOrgIDsByUserID(userID int64, showPrivate bool) ([]int64, error) {
-	orgIDs := make([]int64, 0, 5)
-	sess := x.Table("org_user").Where("uid = ?", userID)
-	if !showPrivate {
-		sess.And("is_public = ?", true)
-	}
-	return orgIDs, sess.Distinct("org_id").Find(&orgIDs)
-}
-
 func getOrgUsersByOrgID(e Engine, orgID int64, limit int) ([]*OrgUser, error) {
 	orgUsers := make([]*OrgUser, 0, 10)
 

internal/db/org_users_test.go

@@ -5,8 +5,10 @@
 package db
 
 import (
+	"context"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"gogs.io/gogs/internal/dbtest"
@@ -43,9 +45,19 @@ func TestOrgUsers(t *testing.T) {
 }
 
 func orgUsersCountByUser(t *testing.T, db *orgUsers) {
+	ctx := context.Background()
+
 	// TODO: Use OrgUsers.Join to replace SQL hack when the method is available.
 	err := db.Exec(`INSERT INTO org_user (uid, org_id) VALUES (?, ?)`, 1, 1).Error
 	require.NoError(t, err)
 	err = db.Exec(`INSERT INTO org_user (uid, org_id) VALUES (?, ?)`, 2, 1).Error
 	require.NoError(t, err)
+
+	got, err := db.CountByUser(ctx, 1)
+	require.NoError(t, err)
+	assert.Equal(t, int64(1), got)
+
+	got, err = db.CountByUser(ctx, 404)
+	require.NoError(t, err)
+	assert.Equal(t, int64(0), got)
 }

internal/db/orgs.go

@@ -0,0 +1,76 @@
+// Copyright 2022 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package db
+
+import (
+	"context"
+
+	"github.com/pkg/errors"
+	"gorm.io/gorm"
+
+	"gogs.io/gogs/internal/dbutil"
+)
+
+// OrgsStore is the persistent interface for organizations.
+//
+// NOTE: All methods are sorted in alphabetical order.
+type OrgsStore interface {
+	// List returns a list of organizations filtered by options.
+	List(ctx context.Context, opts ListOrgOptions) ([]*Organization, error)
+}
+
+var Orgs OrgsStore
+
+var _ OrgsStore = (*orgs)(nil)
+
+type orgs struct {
+	*gorm.DB
+}
+
+// NewOrgsStore returns a persistent interface for orgs with given database
+// connection.
+func NewOrgsStore(db *gorm.DB) OrgsStore {
+	return &orgs{DB: db}
+}
+
+type ListOrgOptions struct {
+	// Filter by the membership with the given user ID.
+	MemberID int64
+	// Whether to include private memberships.
+	IncludePrivateMembers bool
+}
+
+func (db *orgs) List(ctx context.Context, opts ListOrgOptions) ([]*Organization, error) {
+	if opts.MemberID <= 0 {
+		return nil, errors.New("MemberID must be greater than 0")
+	}
+
+	/*
+		Equivalent SQL for PostgreSQL:
+
+		SELECT * FROM "org"
+		JOIN org_user ON org_user.org_id = org.id
+		WHERE
+			org_user.uid = @memberID
+		[AND org_user.is_public = @includePrivateMembers]
+		ORDER BY org.id ASC
+	*/
+	tx := db.WithContext(ctx).
+		Joins(dbutil.Quote("JOIN org_user ON org_user.org_id = %s.id", "user")).
+		Where("org_user.uid = ?", opts.MemberID).
+		Order(dbutil.Quote("%s.id ASC", "user"))
+	if !opts.IncludePrivateMembers {
+		tx = tx.Where("org_user.is_public = ?", true)
+	}
+
+	var orgs []*Organization
+	return orgs, tx.Find(&orgs).Error
+}
+
+type Organization = User
+
+func (o *Organization) TableName() string {
+	return "user"
+}

internal/db/orgs_test.go

@@ -0,0 +1,123 @@
+// Copyright 2022 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package db
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"gogs.io/gogs/internal/dbtest"
+	"gogs.io/gogs/internal/dbutil"
+)
+
+func TestOrgs(t *testing.T) {
+	if testing.Short() {
+		t.Skip()
+	}
+	t.Parallel()
+
+	tables := []interface{}{new(User), new(EmailAddress), new(OrgUser)}
+	db := &orgs{
+		DB: dbtest.NewDB(t, "orgs", tables...),
+	}
+
+	for _, tc := range []struct {
+		name string
+		test func(t *testing.T, db *orgs)
+	}{
+		{"List", orgsList},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Cleanup(func() {
+				err := clearTables(t, db.DB, tables...)
+				require.NoError(t, err)
+			})
+			tc.test(t, db)
+		})
+		if t.Failed() {
+			break
+		}
+	}
+}
+
+func orgsList(t *testing.T, db *orgs) {
+	ctx := context.Background()
+
+	usersStore := NewUsersStore(db.DB)
+	alice, err := usersStore.Create(ctx, "alice", "[email protected]", CreateUserOptions{})
+	require.NoError(t, err)
+	bob, err := usersStore.Create(ctx, "bob", "[email protected]", CreateUserOptions{})
+	require.NoError(t, err)
+
+	// TODO: Use Orgs.Create to replace SQL hack when the method is available.
+	org1, err := usersStore.Create(ctx, "org1", "[email protected]", CreateUserOptions{})
+	require.NoError(t, err)
+	err = db.Exec(
+		dbutil.Quote("UPDATE %s SET %s = ? WHERE id = ?", "user", "type"),
+		UserTypeOrganization, org1.ID,
+	).Error
+	require.NoError(t, err)
+	org2, err := usersStore.Create(ctx, "org2", "[email protected]", CreateUserOptions{})
+	require.NoError(t, err)
+	err = db.Exec(
+		dbutil.Quote("UPDATE %s SET %s = ? WHERE id = ?", "user", "type"),
+		UserTypeOrganization, org2.ID,
+	).Error
+	require.NoError(t, err)
+
+	// TODO: Use OrgUsers.Join to replace SQL hack when the method is available.
+	err = db.Exec(`INSERT INTO org_user (uid, org_id, is_public) VALUES (?, ?, ?)`, alice.ID, org1.ID, false).Error
+	require.NoError(t, err)
+	err = db.Exec(`INSERT INTO org_user (uid, org_id, is_public) VALUES (?, ?, ?)`, alice.ID, org2.ID, true).Error
+	require.NoError(t, err)
+	err = db.Exec(`INSERT INTO org_user (uid, org_id, is_public) VALUES (?, ?, ?)`, bob.ID, org2.ID, true).Error
+	require.NoError(t, err)
+
+	tests := []struct {
+		name         string
+		opts         ListOrgOptions
+		wantOrgNames []string
+	}{
+		{
+			name: "only public memberships for a user",
+			opts: ListOrgOptions{
+				MemberID:              alice.ID,
+				IncludePrivateMembers: false,
+			},
+			wantOrgNames: []string{org2.Name},
+		},
+		{
+			name: "all memberships for a user",
+			opts: ListOrgOptions{
+				MemberID:              alice.ID,
+				IncludePrivateMembers: true,
+			},
+			wantOrgNames: []string{org1.Name, org2.Name},
+		},
+		{
+			name: "no membership for a non-existent user",
+			opts: ListOrgOptions{
+				MemberID:              404,
+				IncludePrivateMembers: true,
+			},
+			wantOrgNames: []string{},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got, err := db.List(ctx, test.opts)
+			require.NoError(t, err)
+
+			gotOrgNames := make([]string, len(got))
+			for i := range got {
+				gotOrgNames[i] = got[i].Name
+			}
+			assert.Equal(t, test.wantOrgNames, gotOrgNames)
+		})
+	}
+}

internal/db/user.go

@@ -60,23 +60,6 @@ func (u *User) getOrganizationCount(e Engine) (int64, error) {
 	return e.Where("uid=?", u.ID).Count(new(OrgUser))
 }
 
-// GetOrganizations returns all organizations that user belongs to.
-func (u *User) GetOrganizations(showPrivate bool) error {
-	orgIDs, err := GetOrgIDsByUserID(u.ID, showPrivate)
-	if err != nil {
-		return fmt.Errorf("GetOrgIDsByUserID: %v", err)
-	}
-	if len(orgIDs) == 0 {
-		return nil
-	}
-
-	u.Orgs = make([]*User, 0, len(orgIDs))
-	if err = x.Where("type = ?", UserTypeOrganization).In("id", orgIDs).Find(&u.Orgs); err != nil {
-		return err
-	}
-	return nil
-}
-
 // IsUserExist checks if given user name exist,
 // the user name should be noncased unique.
 // If uid is presented, then check will rule out that one,

internal/db/users.go

@@ -20,6 +20,7 @@ import (
 	"gogs.io/gogs/internal/auth"
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/cryptoutil"
+	"gogs.io/gogs/internal/dbutil"
 	"gogs.io/gogs/internal/errutil"
 	"gogs.io/gogs/internal/osutil"
 	"gogs.io/gogs/internal/strutil"
@@ -381,16 +382,13 @@ func (db *users) ListFollowers(ctx context.Context, userID int64, page, pageSize
 		LIMIT @limit OFFSET @offset
 	*/
 	users := make([]*User, 0, pageSize)
-	tx := db.WithContext(ctx).
+	return users, db.WithContext(ctx).
+		Joins(dbutil.Quote("LEFT JOIN follow ON follow.user_id = %s.id", "user")).
 		Where("follow.follow_id = ?", userID).
 		Limit(pageSize).Offset((page - 1) * pageSize).
-		Order("follow.id DESC")
-	if conf.UsePostgreSQL {
-		tx.Joins(`LEFT JOIN follow ON follow.user_id = "user".id`)
-	} else {
-		tx.Joins(`LEFT JOIN follow ON follow.user_id = user.id`)
-	}
-	return users, tx.Find(&users).Error
+		Order("follow.id DESC").
+		Find(&users).
+		Error
 }
 
 func (db *users) ListFollowings(ctx context.Context, userID int64, page, pageSize int) ([]*User, error) {
@@ -404,16 +402,13 @@ func (db *users) ListFollowings(ctx context.Context, userID int64, page, pageSiz
 		LIMIT @limit OFFSET @offset
 	*/
 	users := make([]*User, 0, pageSize)
-	tx := db.WithContext(ctx).
+	return users, db.WithContext(ctx).
+		Joins(dbutil.Quote("LEFT JOIN follow ON follow.follow_id = %s.id", "user")).
 		Where("follow.user_id = ?", userID).
 		Limit(pageSize).Offset((page - 1) * pageSize).
-		Order("follow.id DESC")
-	if conf.UsePostgreSQL {
-		tx.Joins(`LEFT JOIN follow ON follow.follow_id = "user".id`)
-	} else {
-		tx.Joins(`LEFT JOIN follow ON follow.follow_id = user.id`)
-	}
-	return users, tx.Find(&users).Error
+		Order("follow.id DESC").
+		Find(&users).
+		Error
 }
 
 func (db *users) UseCustomAvatar(ctx context.Context, userID int64, avatar []byte) error {
@@ -452,7 +447,6 @@ type User struct {
 	LoginSource int64  `xorm:"NOT NULL DEFAULT 0" gorm:"not null;default:0"`
 	LoginName   string
 	Type        UserType
-	Orgs        []*User `xorm:"-" gorm:"-" json:"-"`
 	Location    string
 	Website     string
 	Rands       string `xorm:"VARCHAR(10)" gorm:"type:VARCHAR(10)"`

internal/dbutil/logger.go

@@ -15,5 +15,5 @@ type Logger struct {
 }
 
 func (l *Logger) Printf(format string, args ...interface{}) {
-	fmt.Fprintf(l.Writer, format, args...)
+	_, _ = fmt.Fprintf(l.Writer, format, args...)
 }

internal/dbutil/string.go

@@ -0,0 +1,25 @@
+// Copyright 2022 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package dbutil
+
+import (
+	"fmt"
+
+	"gogs.io/gogs/internal/conf"
+)
+
+// Quote adds surrounding double quotes for all given arguments before being
+// formatted if the current database is UsePostgreSQL.
+func Quote(format string, args ...string) string {
+	anys := make([]any, len(args))
+	for i := range args {
+		if conf.UsePostgreSQL {
+			anys[i] = `"` + args[i] + `"`
+		} else {
+			anys[i] = args[i]
+		}
+	}
+	return fmt.Sprintf(format, anys...)
+}

internal/dbutil/string_test.go

@@ -0,0 +1,25 @@
+// Copyright 2022 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package dbutil
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"gogs.io/gogs/internal/conf"
+)
+
+func TestQuote(t *testing.T) {
+	conf.UsePostgreSQL = true
+	got := Quote("SELECT * FROM %s", "user")
+	want := `SELECT * FROM "user"`
+	assert.Equal(t, want, got)
+	conf.UsePostgreSQL = false
+
+	got = Quote("SELECT * FROM %s", "user")
+	want = `SELECT * FROM user`
+	assert.Equal(t, want, got)
+}

internal/route/api/v1/org/org.go

@@ -43,14 +43,21 @@ func CreateOrgForUser(c *context.APIContext, apiForm api.CreateOrgOption, user *
 }
 
 func listUserOrgs(c *context.APIContext, u *db.User, all bool) {
-	if err := u.GetOrganizations(all); err != nil {
-		c.Error(err, "get organization")
+	orgs, err := db.Orgs.List(
+		c.Req.Context(),
+		db.ListOrgOptions{
+			MemberID:              u.ID,
+			IncludePrivateMembers: all,
+		},
+	)
+	if err != nil {
+		c.Error(err, "list organizations")
 		return
 	}
 
-	apiOrgs := make([]*api.Organization, len(u.Orgs))
-	for i := range u.Orgs {
-		apiOrgs[i] = convert.ToOrganization(u.Orgs[i])
+	apiOrgs := make([]*api.Organization, len(orgs))
+	for i := range orgs {
+		apiOrgs[i] = convert.ToOrganization(orgs[i])
 	}
 	c.JSONSuccess(&apiOrgs)
 }

internal/route/repo/pull.go

@@ -69,11 +69,18 @@ func parseBaseRepository(c *context.Context) *db.Repository {
 	}
 	c.Data["ForkFrom"] = baseRepo.Owner.Name + "/" + baseRepo.Name
 
-	if err := c.User.GetOrganizations(true); err != nil {
-		c.Error(err, "get organizations")
+	orgs, err := db.Orgs.List(
+		c.Req.Context(),
+		db.ListOrgOptions{
+			MemberID:              c.User.ID,
+			IncludePrivateMembers: true,
+		},
+	)
+	if err != nil {
+		c.Error(err, "list organizations")
 		return nil
 	}
-	c.Data["Orgs"] = c.User.Orgs
+	c.Data["Orgs"] = orgs
 
 	return baseRepo
 }

internal/route/user/home.go

@@ -40,11 +40,18 @@ func getDashboardContextUser(c *context.Context) *db.User {
 	}
 	c.Data["ContextUser"] = ctxUser
 
-	if err := c.User.GetOrganizations(true); err != nil {
-		c.Error(err, "get organizations")
+	orgs, err := db.Orgs.List(
+		c.Req.Context(),
+		db.ListOrgOptions{
+			MemberID:              c.User.ID,
+			IncludePrivateMembers: true,
+		},
+	)
+	if err != nil {
+		c.Error(err, "list organizations")
 		return nil
 	}
-	c.Data["Orgs"] = c.User.Orgs
+	c.Data["Orgs"] = orgs
 
 	return ctxUser
 }

templates/user/dashboard/dashboard.tmpl

@@ -87,7 +87,7 @@
 						</div>
 						<div class="ui attached table segment">
 							<ul class="repo-owner-name-list">
-								{{range .ContextUser.Orgs}}
+								{{range .Orgs}}
 									<li>
 										<a href="{{AppSubURL}}/{{.Name}}">
 											<i class="octicon octicon-organization"></i>