diff --git a/.golangci.yml b/.golangci.yml index 631e36925..479ac8125 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -9,16 +9,13 @@ linters-settings: linters: enable: - - deadcode + - unused - errcheck - gosimple - govet - ineffassign - staticcheck - - structcheck - typecheck - - unused - - varcheck - nakedret - gofmt - rowserrcheck diff --git a/internal/context/repo.go b/internal/context/repo.go index 3b4b32818..f5d4d5c5a 100644 --- a/internal/context/repo.go +++ b/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 { diff --git a/internal/db/user.go b/internal/db/user.go index 97e2800f7..08b7e09eb 100644 --- a/internal/db/user.go +++ b/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 diff --git a/internal/db/user_cache.go b/internal/db/user_cache.go index 314ea3b18..f4fa81727 100644 --- a/internal/db/user_cache.go +++ b/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) } diff --git a/internal/db/users.go b/internal/db/users.go index 8cebd8148..d52cfc4c8 100644 --- a/internal/db/users.go +++ b/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 +} diff --git a/internal/db/users_test.go b/internal/db/users_test.go index 94922a180..68c1bfdcc 100644 --- a/internal/db/users_test.go +++ b/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) +} diff --git a/internal/route/lfs/mocks_test.go b/internal/route/lfs/mocks_test.go index 0b7d010bd..57bc9fbf4 100644 --- a/internal/route/lfs/mocks_test.go +++ b/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} +} diff --git a/internal/route/repo/issue.go b/internal/route/repo/issue.go index 5caa16cba..cf59856eb 100644 --- a/internal/route/repo/issue.go +++ b/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 }