Browse Source

refactor(db): migrate methods off `user.go` (#7331)

Joe Chen 2 years ago
parent
commit
cc4d4eacad

+ 12 - 0
internal/db/issue.go

@@ -79,6 +79,18 @@ func (issue *Issue) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
+// Deprecated: Use Users.GetByID instead.
+func getUserByID(e Engine, id int64) (*User, error) {
+	u := new(User)
+	has, err := e.ID(id).Get(u)
+	if err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrUserNotExist{args: errutil.Args{"userID": id}}
+	}
+	return u, nil
+}
+
 func (issue *Issue) loadAttributes(e Engine) (err error) {
 	if issue.Repo == nil {
 		issue.Repo, err = getRepositoryByID(e, issue.RepoID)

+ 10 - 0
internal/db/orgs.go

@@ -19,6 +19,12 @@ import (
 type OrgsStore interface {
 	// List returns a list of organizations filtered by options.
 	List(ctx context.Context, opts ListOrgsOptions) ([]*Organization, error)
+	// SearchByName returns a list of organizations whose username or full name
+	// matches the given keyword case-insensitively. Results are paginated by given
+	// page and page size, and sorted by the given order (e.g. "id DESC"). A total
+	// count of all results is also returned. If the order is not given, it's up to
+	// the database to decide.
+	SearchByName(ctx context.Context, keyword string, page, pageSize int, orderBy string) ([]*Organization, int64, error)
 }
 
 var Orgs OrgsStore
@@ -69,6 +75,10 @@ func (db *orgs) List(ctx context.Context, opts ListOrgsOptions) ([]*Organization
 	return orgs, tx.Find(&orgs).Error
 }
 
+func (db *orgs) SearchByName(ctx context.Context, keyword string, page, pageSize int, orderBy string) ([]*Organization, int64, error) {
+	return searchUserByName(ctx, db.DB, UserTypeOrganization, keyword, page, pageSize, orderBy)
+}
+
 type Organization = User
 
 func (o *Organization) TableName() string {

+ 50 - 7
internal/db/orgs_test.go

@@ -31,6 +31,7 @@ func TestOrgs(t *testing.T) {
 		test func(t *testing.T, db *orgs)
 	}{
 		{"List", orgsList},
+		{"SearchByName", orgsSearchByName},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Cleanup(func() {
@@ -57,16 +58,11 @@ func orgsList(t *testing.T, db *orgs) {
 	// 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 type = ? WHERE id = ?", "user"),
-		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 type = ? WHERE id = ?", "user"),
-		UserTypeOrganization, org2.ID,
+		dbutil.Quote("UPDATE %s SET type = ? WHERE id IN (?, ?)", "user"),
+		UserTypeOrganization, org1.ID, org2.ID,
 	).Error
 	require.NoError(t, err)
 
@@ -121,3 +117,50 @@ func orgsList(t *testing.T, db *orgs) {
 		})
 	}
 }
+
+func orgsSearchByName(t *testing.T, db *orgs) {
+	ctx := context.Background()
+
+	// TODO: Use Orgs.Create to replace SQL hack when the method is available.
+	usersStore := NewUsersStore(db.DB)
+	org1, err := usersStore.Create(ctx, "org1", "[email protected]", CreateUserOptions{FullName: "Acme Corp"})
+	require.NoError(t, err)
+	org2, err := usersStore.Create(ctx, "org2", "[email protected]", CreateUserOptions{FullName: "Acme Corp 2"})
+	require.NoError(t, err)
+	err = db.Exec(
+		dbutil.Quote("UPDATE %s SET type = ? WHERE id IN (?, ?)", "user"),
+		UserTypeOrganization, org1.ID, org2.ID,
+	).Error
+	require.NoError(t, err)
+
+	t.Run("search for username org1", func(t *testing.T) {
+		orgs, count, err := db.SearchByName(ctx, "G1", 1, 1, "")
+		require.NoError(t, err)
+		require.Len(t, orgs, int(count))
+		assert.Equal(t, int64(1), count)
+		assert.Equal(t, org1.ID, orgs[0].ID)
+	})
+
+	t.Run("search for username org2", func(t *testing.T) {
+		orgs, count, err := db.SearchByName(ctx, "G2", 1, 1, "")
+		require.NoError(t, err)
+		require.Len(t, orgs, int(count))
+		assert.Equal(t, int64(1), count)
+		assert.Equal(t, org2.ID, orgs[0].ID)
+	})
+
+	t.Run("search for full name acme", func(t *testing.T) {
+		orgs, count, err := db.SearchByName(ctx, "ACME", 1, 10, "")
+		require.NoError(t, err)
+		require.Len(t, orgs, int(count))
+		assert.Equal(t, int64(2), count)
+	})
+
+	t.Run("search for full name acme ORDER BY id DESC LIMIT 1", func(t *testing.T) {
+		orgs, count, err := db.SearchByName(ctx, "ACME", 1, 1, "id DESC")
+		require.NoError(t, err)
+		require.Len(t, orgs, 1)
+		assert.Equal(t, int64(2), count)
+		assert.Equal(t, org2.ID, orgs[0].ID)
+	})
+}

+ 0 - 55
internal/db/user.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	_ "image/jpeg"
 	"os"
-	"strings"
 	"time"
 
 	log "unknwon.dev/clog/v2"
@@ -17,7 +16,6 @@ import (
 
 	"github.com/gogs/git-module"
 
-	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/repoutil"
 	"gogs.io/gogs/internal/userutil"
 )
@@ -202,17 +200,6 @@ func DeleteInactivateUsers() (err error) {
 	return err
 }
 
-func getUserByID(e Engine, id int64) (*User, error) {
-	u := new(User)
-	has, err := e.ID(id).Get(u)
-	if err != nil {
-		return nil, err
-	} else if !has {
-		return nil, ErrUserNotExist{args: map[string]any{"userID": id}}
-	}
-	return u, nil
-}
-
 // GetUserEmailsByNames returns a list of e-mails corresponds to names.
 func GetUserEmailsByNames(names []string) []string {
 	mails := make([]string, 0, len(names))
@@ -264,48 +251,6 @@ func ValidateCommitsWithEmails(oldCommits []*git.Commit) []*UserCommit {
 	return newCommits
 }
 
-type SearchUserOptions struct {
-	Keyword  string
-	Type     UserType
-	OrderBy  string
-	Page     int
-	PageSize int // Can be smaller than or equal to setting.UI.ExplorePagingNum
-}
-
-// SearchUserByName takes keyword and part of user name to search,
-// it returns results in given range and number of total results.
-func SearchUserByName(opts *SearchUserOptions) (users []*User, _ int64, _ error) {
-	if opts.Keyword == "" {
-		return users, 0, nil
-	}
-	opts.Keyword = strings.ToLower(opts.Keyword)
-
-	if opts.PageSize <= 0 || opts.PageSize > conf.UI.ExplorePagingNum {
-		opts.PageSize = conf.UI.ExplorePagingNum
-	}
-	if opts.Page <= 0 {
-		opts.Page = 1
-	}
-
-	searchQuery := "%" + opts.Keyword + "%"
-	users = make([]*User, 0, opts.PageSize)
-	// Append conditions
-	sess := x.Where("LOWER(lower_name) LIKE ?", searchQuery).
-		Or("LOWER(full_name) LIKE ?", searchQuery).
-		And("type = ?", opts.Type)
-
-	countSess := *sess
-	count, err := countSess.Count(new(User))
-	if err != nil {
-		return nil, 0, fmt.Errorf("Count: %v", err)
-	}
-
-	if len(opts.OrderBy) > 0 {
-		sess.OrderBy(opts.OrderBy)
-	}
-	return users, count, sess.Limit(opts.PageSize, (opts.Page-1)*opts.PageSize).Find(&users)
-}
-
 // GetRepositoryAccesses finds all repositories with their access mode where a user has access but does not own.
 func (u *User) GetRepositoryAccesses() (map[*Repository]AccessMode, error) {
 	accesses := make([]*Access, 0, 10)

+ 29 - 0
internal/db/users.go

@@ -90,6 +90,12 @@ type UsersStore interface {
 	// Results are paginated by given page and page size, and sorted by the time of
 	// follow in descending order.
 	ListFollowings(ctx context.Context, userID int64, page, pageSize int) ([]*User, error)
+	// SearchByName returns a list of users whose username or full name matches the
+	// given keyword case-insensitively. Results are paginated by given page and
+	// page size, and sorted by the given order (e.g. "id DESC"). A total count of
+	// all results is also returned. If the order is not given, it's up to the
+	// database to decide.
+	SearchByName(ctx context.Context, keyword string, page, pageSize int, orderBy string) ([]*User, int64, error)
 	// Update updates fields for the given user.
 	Update(ctx context.Context, userID int64, opts UpdateUserOptions) error
 	// UseCustomAvatar uses the given avatar as the user custom avatar.
@@ -570,6 +576,29 @@ func (db *users) ListFollowings(ctx context.Context, userID int64, page, pageSiz
 		Error
 }
 
+func searchUserByName(ctx context.Context, db *gorm.DB, userType UserType, keyword string, page, pageSize int, orderBy string) ([]*User, int64, error) {
+	if keyword == "" {
+		return []*User{}, 0, nil
+	}
+	keyword = "%" + strings.ToLower(keyword) + "%"
+
+	tx := db.WithContext(ctx).
+		Where("type = ? AND (lower_name LIKE ? OR LOWER(full_name) LIKE ?)", userType, keyword, keyword)
+
+	var count int64
+	err := tx.Model(&User{}).Count(&count).Error
+	if err != nil {
+		return nil, 0, errors.Wrap(err, "count")
+	}
+
+	users := make([]*User, 0, pageSize)
+	return users, count, tx.Order(orderBy).Limit(pageSize).Offset((page - 1) * pageSize).Find(&users).Error
+}
+
+func (db *users) SearchByName(ctx context.Context, keyword string, page, pageSize int, orderBy string) ([]*User, int64, error) {
+	return searchUserByName(ctx, db.DB, UserTypeIndividual, keyword, page, pageSize, orderBy)
+}
+
 type UpdateUserOptions struct {
 	LoginSource *int64
 	LoginName   *string

+ 41 - 0
internal/db/users_test.go

@@ -105,6 +105,7 @@ func TestUsers(t *testing.T) {
 		{"List", usersList},
 		{"ListFollowers", usersListFollowers},
 		{"ListFollowings", usersListFollowings},
+		{"SearchByName", usersSearchByName},
 		{"Update", usersUpdate},
 		{"UseCustomAvatar", usersUseCustomAvatar},
 	} {
@@ -756,6 +757,46 @@ func usersListFollowings(t *testing.T, db *users) {
 	assert.Equal(t, alice.ID, got[0].ID)
 }
 
+func usersSearchByName(t *testing.T, db *users) {
+	ctx := context.Background()
+
+	alice, err := db.Create(ctx, "alice", "[email protected]", CreateUserOptions{FullName: "Alice Jordan"})
+	require.NoError(t, err)
+	bob, err := db.Create(ctx, "bob", "[email protected]", CreateUserOptions{FullName: "Bob Jordan"})
+	require.NoError(t, err)
+
+	t.Run("search for username alice", func(t *testing.T) {
+		users, count, err := db.SearchByName(ctx, "Li", 1, 1, "")
+		require.NoError(t, err)
+		require.Len(t, users, int(count))
+		assert.Equal(t, int64(1), count)
+		assert.Equal(t, alice.ID, users[0].ID)
+	})
+
+	t.Run("search for username bob", func(t *testing.T) {
+		users, count, err := db.SearchByName(ctx, "oB", 1, 1, "")
+		require.NoError(t, err)
+		require.Len(t, users, int(count))
+		assert.Equal(t, int64(1), count)
+		assert.Equal(t, bob.ID, users[0].ID)
+	})
+
+	t.Run("search for full name jordan", func(t *testing.T) {
+		users, count, err := db.SearchByName(ctx, "Jo", 1, 10, "")
+		require.NoError(t, err)
+		require.Len(t, users, int(count))
+		assert.Equal(t, int64(2), count)
+	})
+
+	t.Run("search for full name jordan ORDER BY id DESC LIMIT 1", func(t *testing.T) {
+		users, count, err := db.SearchByName(ctx, "Jo", 1, 1, "id DESC")
+		require.NoError(t, err)
+		require.Len(t, users, 1)
+		assert.Equal(t, int64(2), count)
+		assert.Equal(t, bob.ID, users[0].ID)
+	})
+}
+
 func usersUpdate(t *testing.T, db *users) {
 	ctx := context.Background()
 

+ 4 - 11
internal/route/api/v1/user/user.go

@@ -7,8 +7,6 @@ package user
 import (
 	"net/http"
 
-	"github.com/unknwon/com"
-
 	api "github.com/gogs/go-gogs-client"
 
 	"gogs.io/gogs/internal/context"
@@ -17,16 +15,11 @@ import (
 )
 
 func Search(c *context.APIContext) {
-	opts := &db.SearchUserOptions{
-		Keyword:  c.Query("q"),
-		Type:     db.UserTypeIndividual,
-		PageSize: com.StrTo(c.Query("limit")).MustInt(),
-	}
-	if opts.PageSize == 0 {
-		opts.PageSize = 10
+	pageSize := c.QueryInt("limit")
+	if pageSize <= 0 {
+		pageSize = 10
 	}
-
-	users, _, err := db.SearchUserByName(opts)
+	users, _, err := db.Users.SearchByName(c.Req.Context(), c.Query("q"), 1, pageSize, "")
 	if err != nil {
 		c.JSON(http.StatusInternalServerError, map[string]any{
 			"ok":    false,

+ 6 - 8
internal/route/home.go

@@ -113,15 +113,13 @@ func RenderUserSearch(c *context.Context, opts *UserSearchOptions) {
 		}
 		count = opts.Counter(c.Req.Context())
 	} else {
-		users, count, err = db.SearchUserByName(&db.SearchUserOptions{
-			Keyword:  keyword,
-			Type:     opts.Type,
-			OrderBy:  opts.OrderBy,
-			Page:     page,
-			PageSize: opts.PageSize,
-		})
+		search := db.Users.SearchByName
+		if opts.Type == db.UserTypeOrganization {
+			search = db.Orgs.SearchByName
+		}
+		users, count, err = search(c.Req.Context(), keyword, page, opts.PageSize, opts.OrderBy)
 		if err != nil {
-			c.Error(err, "search user by name")
+			c.Error(err, "search by name")
 			return
 		}
 	}

+ 136 - 0
internal/route/lfs/mocks_test.go

@@ -2334,6 +2334,9 @@ type MockUsersStore struct {
 	// ListFollowingsFunc is an instance of a mock function object
 	// controlling the behavior of the method ListFollowings.
 	ListFollowingsFunc *UsersStoreListFollowingsFunc
+	// SearchByNameFunc is an instance of a mock function object controlling
+	// the behavior of the method SearchByName.
+	SearchByNameFunc *UsersStoreSearchByNameFunc
 	// UpdateFunc is an instance of a mock function object controlling the
 	// behavior of the method Update.
 	UpdateFunc *UsersStoreUpdateFunc
@@ -2416,6 +2419,11 @@ func NewMockUsersStore() *MockUsersStore {
 				return
 			},
 		},
+		SearchByNameFunc: &UsersStoreSearchByNameFunc{
+			defaultHook: func(context.Context, string, int, int, string) (r0 []*db.User, r1 int64, r2 error) {
+				return
+			},
+		},
 		UpdateFunc: &UsersStoreUpdateFunc{
 			defaultHook: func(context.Context, int64, db.UpdateUserOptions) (r0 error) {
 				return
@@ -2503,6 +2511,11 @@ func NewStrictMockUsersStore() *MockUsersStore {
 				panic("unexpected invocation of MockUsersStore.ListFollowings")
 			},
 		},
+		SearchByNameFunc: &UsersStoreSearchByNameFunc{
+			defaultHook: func(context.Context, string, int, int, string) ([]*db.User, int64, error) {
+				panic("unexpected invocation of MockUsersStore.SearchByName")
+			},
+		},
 		UpdateFunc: &UsersStoreUpdateFunc{
 			defaultHook: func(context.Context, int64, db.UpdateUserOptions) error {
 				panic("unexpected invocation of MockUsersStore.Update")
@@ -2562,6 +2575,9 @@ func NewMockUsersStoreFrom(i db.UsersStore) *MockUsersStore {
 		ListFollowingsFunc: &UsersStoreListFollowingsFunc{
 			defaultHook: i.ListFollowings,
 		},
+		SearchByNameFunc: &UsersStoreSearchByNameFunc{
+			defaultHook: i.SearchByName,
+		},
 		UpdateFunc: &UsersStoreUpdateFunc{
 			defaultHook: i.Update,
 		},
@@ -4101,6 +4117,126 @@ func (c UsersStoreListFollowingsFuncCall) Results() []interface{} {
 	return []interface{}{c.Result0, c.Result1}
 }
 
+// UsersStoreSearchByNameFunc describes the behavior when the SearchByName
+// method of the parent MockUsersStore instance is invoked.
+type UsersStoreSearchByNameFunc struct {
+	defaultHook func(context.Context, string, int, int, string) ([]*db.User, int64, error)
+	hooks       []func(context.Context, string, int, int, string) ([]*db.User, int64, error)
+	history     []UsersStoreSearchByNameFuncCall
+	mutex       sync.Mutex
+}
+
+// SearchByName delegates to the next hook function in the queue and stores
+// the parameter and result values of this invocation.
+func (m *MockUsersStore) SearchByName(v0 context.Context, v1 string, v2 int, v3 int, v4 string) ([]*db.User, int64, error) {
+	r0, r1, r2 := m.SearchByNameFunc.nextHook()(v0, v1, v2, v3, v4)
+	m.SearchByNameFunc.appendCall(UsersStoreSearchByNameFuncCall{v0, v1, v2, v3, v4, r0, r1, r2})
+	return r0, r1, r2
+}
+
+// SetDefaultHook sets function that is called when the SearchByName method
+// of the parent MockUsersStore instance is invoked and the hook queue is
+// empty.
+func (f *UsersStoreSearchByNameFunc) SetDefaultHook(hook func(context.Context, string, int, int, string) ([]*db.User, int64, error)) {
+	f.defaultHook = hook
+}
+
+// PushHook adds a function to the end of hook queue. Each invocation of the
+// SearchByName method of the parent MockUsersStore instance invokes the
+// hook at the front of the queue and discards it. After the queue is empty,
+// the default hook function is invoked for any future action.
+func (f *UsersStoreSearchByNameFunc) PushHook(hook func(context.Context, string, int, int, string) ([]*db.User, int64, error)) {
+	f.mutex.Lock()
+	f.hooks = append(f.hooks, hook)
+	f.mutex.Unlock()
+}
+
+// SetDefaultReturn calls SetDefaultHook with a function that returns the
+// given values.
+func (f *UsersStoreSearchByNameFunc) SetDefaultReturn(r0 []*db.User, r1 int64, r2 error) {
+	f.SetDefaultHook(func(context.Context, string, int, int, string) ([]*db.User, int64, error) {
+		return r0, r1, r2
+	})
+}
+
+// PushReturn calls PushHook with a function that returns the given values.
+func (f *UsersStoreSearchByNameFunc) PushReturn(r0 []*db.User, r1 int64, r2 error) {
+	f.PushHook(func(context.Context, string, int, int, string) ([]*db.User, int64, error) {
+		return r0, r1, r2
+	})
+}
+
+func (f *UsersStoreSearchByNameFunc) nextHook() func(context.Context, string, int, int, string) ([]*db.User, int64, error) {
+	f.mutex.Lock()
+	defer f.mutex.Unlock()
+
+	if len(f.hooks) == 0 {
+		return f.defaultHook
+	}
+
+	hook := f.hooks[0]
+	f.hooks = f.hooks[1:]
+	return hook
+}
+
+func (f *UsersStoreSearchByNameFunc) appendCall(r0 UsersStoreSearchByNameFuncCall) {
+	f.mutex.Lock()
+	f.history = append(f.history, r0)
+	f.mutex.Unlock()
+}
+
+// History returns a sequence of UsersStoreSearchByNameFuncCall objects
+// describing the invocations of this function.
+func (f *UsersStoreSearchByNameFunc) History() []UsersStoreSearchByNameFuncCall {
+	f.mutex.Lock()
+	history := make([]UsersStoreSearchByNameFuncCall, len(f.history))
+	copy(history, f.history)
+	f.mutex.Unlock()
+
+	return history
+}
+
+// UsersStoreSearchByNameFuncCall is an object that describes an invocation
+// of method SearchByName on an instance of MockUsersStore.
+type UsersStoreSearchByNameFuncCall struct {
+	// Arg0 is the value of the 1st argument passed to this method
+	// invocation.
+	Arg0 context.Context
+	// Arg1 is the value of the 2nd argument passed to this method
+	// invocation.
+	Arg1 string
+	// Arg2 is the value of the 3rd argument passed to this method
+	// invocation.
+	Arg2 int
+	// Arg3 is the value of the 4th argument passed to this method
+	// invocation.
+	Arg3 int
+	// Arg4 is the value of the 5th argument passed to this method
+	// invocation.
+	Arg4 string
+	// Result0 is the value of the 1st result returned from this method
+	// invocation.
+	Result0 []*db.User
+	// Result1 is the value of the 2nd result returned from this method
+	// invocation.
+	Result1 int64
+	// Result2 is the value of the 3rd result returned from this method
+	// invocation.
+	Result2 error
+}
+
+// Args returns an interface slice containing the arguments of this
+// invocation.
+func (c UsersStoreSearchByNameFuncCall) Args() []interface{} {
+	return []interface{}{c.Arg0, c.Arg1, c.Arg2, c.Arg3, c.Arg4}
+}
+
+// Results returns an interface slice containing the results of this
+// invocation.
+func (c UsersStoreSearchByNameFuncCall) Results() []interface{} {
+	return []interface{}{c.Result0, c.Result1, c.Result2}
+}
+
 // UsersStoreUpdateFunc describes the behavior when the Update method of the
 // parent MockUsersStore instance is invoked.
 type UsersStoreUpdateFunc struct {