refactor(db): move `User.HasForkedRepository` to `users.HasForkedRepository` (#7176)

Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>

.golangci.yml

@@ -9,16 +9,13 @@ linters-settings:
 
 linters:
   enable:
-    - deadcode
+    - unused
     - errcheck
     - gosimple
     - govet
     - ineffassign
     - staticcheck
-    - structcheck
     - typecheck
-    - unused
-    - varcheck
     - nakedret
     - gofmt
     - rowserrcheck

internal/context/repo.go

@@ -403,7 +403,7 @@ func RepoRef() macaron.Handler {
 		c.Data["IsViewCommit"] = c.Repo.IsViewCommit
 
 		// People who have push access or have forked repository can propose a new pull request.
-		if c.Repo.IsWriter() || (c.IsLogged && c.User.HasForkedRepo(c.Repo.Repository.ID)) {
+		if c.Repo.IsWriter() || (c.IsLogged && db.Users.HasForkedRepository(c.Req.Context(), c.User.ID, c.Repo.Repository.ID)) {
 			// Pull request is allowed if this is a fork repository
 			// and base repository accepts pull requests.
 			if c.Repo.Repository.BaseRepo != nil {

internal/db/user.go

@@ -124,11 +124,6 @@ func (u *User) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
-// IDStr returns string representation of user's ID.
-func (u *User) IDStr() string {
-	return com.ToStr(u.ID)
-}
-
 func (u *User) APIFormat() *api.User {
 	return &api.User{
 		ID:        u.ID,
@@ -140,17 +135,6 @@ func (u *User) APIFormat() *api.User {
 	}
 }
 
-// returns true if user login type is LoginPlain.
-func (u *User) IsLocal() bool {
-	return u.LoginSource <= 0
-}
-
-// HasForkedRepo checks if user has already forked a repository with given ID.
-func (u *User) HasForkedRepo(repoID int64) bool {
-	_, has, _ := HasForkedRepo(u.ID, repoID)
-	return has
-}
-
 func (u *User) RepoCreationNum() int {
 	if u.MaxRepoCreation <= -1 {
 		return conf.Repository.MaxCreationLimit

internal/db/user_cache.go

@@ -4,13 +4,17 @@
 
 package db
 
+import (
+	"fmt"
+)
+
 // MailResendCacheKey returns key used for cache mail resend.
 func (u *User) MailResendCacheKey() string {
-	return "MailResend_" + u.IDStr()
+	return fmt.Sprintf("MailResend_%d", u.ID)
 }
 
 // TwoFactorCacheKey returns key used for cache two factor passcode.
 // e.g. TwoFactor_1_012664
 func (u *User) TwoFactorCacheKey(passcode string) string {
-	return "TwoFactor_" + u.IDStr() + "_" + passcode
+	return fmt.Sprintf("TwoFactor_%d_%s", u.ID, passcode)
 }

internal/db/users.go

@@ -48,6 +48,8 @@ type UsersStore interface {
 	// GetByUsername returns the user with given username. It returns
 	// ErrUserNotExist when not found.
 	GetByUsername(ctx context.Context, username string) (*User, error)
+	// HasForkedRepository returns true if the user has forked given repository.
+	HasForkedRepository(ctx context.Context, userID, repoID int64) bool
 }
 
 var Users UsersStore
@@ -68,6 +70,11 @@ func (u *User) AfterFind(_ *gorm.DB) error {
 	return nil
 }
 
+// IsLocal returns true if user is created as local account.
+func (u *User) IsLocal() bool {
+	return u.LoginSource <= 0
+}
+
 var _ UsersStore = (*users)(nil)
 
 type users struct {
@@ -344,3 +351,9 @@ func (db *users) GetByUsername(ctx context.Context, username string) (*User, err
 	}
 	return user, nil
 }
+
+func (db *users) HasForkedRepository(ctx context.Context, userID, repoID int64) bool {
+	var count int64
+	db.WithContext(ctx).Model(new(Repository)).Where("owner_id = ? AND fork_id = ?", userID, repoID).Count(&count)
+	return count > 0
+}

internal/db/users_test.go

@@ -24,7 +24,7 @@ func TestUsers(t *testing.T) {
 	}
 	t.Parallel()
 
-	tables := []interface{}{new(User), new(EmailAddress)}
+	tables := []interface{}{new(User), new(EmailAddress), new(Repository)}
 	db := &users{
 		DB: dbtest.NewDB(t, "users", tables...),
 	}
@@ -38,6 +38,7 @@ func TestUsers(t *testing.T) {
 		{"GetByEmail", usersGetByEmail},
 		{"GetByID", usersGetByID},
 		{"GetByUsername", usersGetByUsername},
+		{"HasForkedRepository", usersHasForkedRepository},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Cleanup(func() {
@@ -275,3 +276,23 @@ func usersGetByUsername(t *testing.T, db *users) {
 	wantErr := ErrUserNotExist{args: errutil.Args{"name": "bad_username"}}
 	assert.Equal(t, wantErr, err)
 }
+
+func usersHasForkedRepository(t *testing.T, db *users) {
+	ctx := context.Background()
+
+	has := db.HasForkedRepository(ctx, 1, 1)
+	assert.False(t, has)
+
+	_, err := NewReposStore(db.DB).Create(
+		ctx,
+		1,
+		CreateRepoOptions{
+			Name:   "repo1",
+			ForkID: 1,
+		},
+	)
+	require.NoError(t, err)
+
+	has = db.HasForkedRepository(ctx, 1, 1)
+	assert.True(t, has)
+}

internal/route/lfs/mocks_test.go

@@ -2308,6 +2308,9 @@ type MockUsersStore struct {
 	// GetByUsernameFunc is an instance of a mock function object
 	// controlling the behavior of the method GetByUsername.
 	GetByUsernameFunc *UsersStoreGetByUsernameFunc
+	// HasForkedRepositoryFunc is an instance of a mock function object
+	// controlling the behavior of the method HasForkedRepository.
+	HasForkedRepositoryFunc *UsersStoreHasForkedRepositoryFunc
 }
 
 // NewMockUsersStore creates a new mock of the UsersStore interface. All
@@ -2339,6 +2342,11 @@ func NewMockUsersStore() *MockUsersStore {
 				return
 			},
 		},
+		HasForkedRepositoryFunc: &UsersStoreHasForkedRepositoryFunc{
+			defaultHook: func(context.Context, int64, int64) (r0 bool) {
+				return
+			},
+		},
 	}
 }
 
@@ -2371,6 +2379,11 @@ func NewStrictMockUsersStore() *MockUsersStore {
 				panic("unexpected invocation of MockUsersStore.GetByUsername")
 			},
 		},
+		HasForkedRepositoryFunc: &UsersStoreHasForkedRepositoryFunc{
+			defaultHook: func(context.Context, int64, int64) bool {
+				panic("unexpected invocation of MockUsersStore.HasForkedRepository")
+			},
+		},
 	}
 }
 
@@ -2393,6 +2406,9 @@ func NewMockUsersStoreFrom(i db.UsersStore) *MockUsersStore {
 		GetByUsernameFunc: &UsersStoreGetByUsernameFunc{
 			defaultHook: i.GetByUsername,
 		},
+		HasForkedRepositoryFunc: &UsersStoreHasForkedRepositoryFunc{
+			defaultHook: i.HasForkedRepository,
+		},
 	}
 }
 
@@ -2946,3 +2962,113 @@ func (c UsersStoreGetByUsernameFuncCall) Args() []interface{} {
 func (c UsersStoreGetByUsernameFuncCall) Results() []interface{} {
 	return []interface{}{c.Result0, c.Result1}
 }
+
+// UsersStoreHasForkedRepositoryFunc describes the behavior when the
+// HasForkedRepository method of the parent MockUsersStore instance is
+// invoked.
+type UsersStoreHasForkedRepositoryFunc struct {
+	defaultHook func(context.Context, int64, int64) bool
+	hooks       []func(context.Context, int64, int64) bool
+	history     []UsersStoreHasForkedRepositoryFuncCall
+	mutex       sync.Mutex
+}
+
+// HasForkedRepository delegates to the next hook function in the queue and
+// stores the parameter and result values of this invocation.
+func (m *MockUsersStore) HasForkedRepository(v0 context.Context, v1 int64, v2 int64) bool {
+	r0 := m.HasForkedRepositoryFunc.nextHook()(v0, v1, v2)
+	m.HasForkedRepositoryFunc.appendCall(UsersStoreHasForkedRepositoryFuncCall{v0, v1, v2, r0})
+	return r0
+}
+
+// SetDefaultHook sets function that is called when the HasForkedRepository
+// method of the parent MockUsersStore instance is invoked and the hook
+// queue is empty.
+func (f *UsersStoreHasForkedRepositoryFunc) SetDefaultHook(hook func(context.Context, int64, int64) bool) {
+	f.defaultHook = hook
+}
+
+// PushHook adds a function to the end of hook queue. Each invocation of the
+// HasForkedRepository 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 *UsersStoreHasForkedRepositoryFunc) PushHook(hook func(context.Context, int64, int64) bool) {
+	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 *UsersStoreHasForkedRepositoryFunc) SetDefaultReturn(r0 bool) {
+	f.SetDefaultHook(func(context.Context, int64, int64) bool {
+		return r0
+	})
+}
+
+// PushReturn calls PushHook with a function that returns the given values.
+func (f *UsersStoreHasForkedRepositoryFunc) PushReturn(r0 bool) {
+	f.PushHook(func(context.Context, int64, int64) bool {
+		return r0
+	})
+}
+
+func (f *UsersStoreHasForkedRepositoryFunc) nextHook() func(context.Context, int64, int64) bool {
+	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 *UsersStoreHasForkedRepositoryFunc) appendCall(r0 UsersStoreHasForkedRepositoryFuncCall) {
+	f.mutex.Lock()
+	f.history = append(f.history, r0)
+	f.mutex.Unlock()
+}
+
+// History returns a sequence of UsersStoreHasForkedRepositoryFuncCall
+// objects describing the invocations of this function.
+func (f *UsersStoreHasForkedRepositoryFunc) History() []UsersStoreHasForkedRepositoryFuncCall {
+	f.mutex.Lock()
+	history := make([]UsersStoreHasForkedRepositoryFuncCall, len(f.history))
+	copy(history, f.history)
+	f.mutex.Unlock()
+
+	return history
+}
+
+// UsersStoreHasForkedRepositoryFuncCall is an object that describes an
+// invocation of method HasForkedRepository on an instance of
+// MockUsersStore.
+type UsersStoreHasForkedRepositoryFuncCall 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 int64
+	// Arg2 is the value of the 3rd argument passed to this method
+	// invocation.
+	Arg2 int64
+	// Result0 is the value of the 1st result returned from this method
+	// invocation.
+	Result0 bool
+}
+
+// Args returns an interface slice containing the arguments of this
+// invocation.
+func (c UsersStoreHasForkedRepositoryFuncCall) Args() []interface{} {
+	return []interface{}{c.Arg0, c.Arg1, c.Arg2}
+}
+
+// Results returns an interface slice containing the results of this
+// invocation.
+func (c UsersStoreHasForkedRepositoryFuncCall) Results() []interface{} {
+	return []interface{}{c.Result0}
+}

internal/route/repo/issue.go

@@ -68,7 +68,7 @@ func MustAllowPulls(c *context.Context) {
 	}
 
 	// User can send pull request if owns a forked repository.
-	if c.IsLogged && c.User.HasForkedRepo(c.Repo.Repository.ID) {
+	if c.IsLogged && db.Users.HasForkedRepository(c.Req.Context(), c.User.ID, c.Repo.Repository.ID) {
 		c.Repo.PullRequest.Allowed = true
 		c.Repo.PullRequest.HeadInfo = c.User.Name + ":" + c.Repo.BranchName
 	}