From 95d3266bee797cbeb7228d361fe32531737906d2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 17 Nov 2021 17:58:31 +0800 Subject: [PATCH] Move user follow and openid into models/user/ (#17613) * Move UserRedirect into models/user/ * Fix lint & test * Fix lint * Fix lint * remove nolint comment * Fix lint * Move user follow and openid into models/user * Ignore the lint * Ignore the lint * Fix test * ignore stutters lint on UserOpenID --- .golangci.yml | 3 ++ integrations/delete_user_test.go | 5 +- models/error.go | 15 ------ models/statistic.go | 3 +- models/user.go | 37 ++++++++++++-- models/{user_follow.go => user/follow.go} | 2 +- models/user/follow_test.go | 22 ++++++++ models/user/main_test.go | 2 + models/{user_openid.go => user/openid.go} | 49 +++++++----------- .../openid_test.go} | 21 +------- models/user_follow_test.go | 50 ------------------- models/user_test.go | 48 ++++++++++++++++++ routers/api/v1/user/follower.go | 5 +- routers/web/user/auth_openid.go | 13 ++--- routers/web/user/profile.go | 6 +-- routers/web/user/setting/security.go | 3 +- routers/web/user/setting/security_openid.go | 14 +++--- 17 files changed, 155 insertions(+), 143 deletions(-) rename models/{user_follow.go => user/follow.go} (99%) create mode 100644 models/user/follow_test.go rename models/{user_openid.go => user/openid.go} (77%) rename models/{user_openid_test.go => user/openid_test.go} (75%) delete mode 100644 models/user_follow_test.go diff --git a/.golangci.yml b/.golangci.yml index 1065776a14..9620dcaa35 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -111,3 +111,6 @@ issues: linters: - staticcheck text: "svc.IsAnInteractiveSession is deprecated: Use IsWindowsService instead." + - path: models/user/openid.go + linters: + - golint diff --git a/integrations/delete_user_test.go b/integrations/delete_user_test.go index 86896c8ae1..f8efab0a24 100644 --- a/integrations/delete_user_test.go +++ b/integrations/delete_user_test.go @@ -11,12 +11,13 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" ) func assertUserDeleted(t *testing.T, userID int64) { unittest.AssertNotExistsBean(t, &models.User{ID: userID}) - unittest.AssertNotExistsBean(t, &models.Follow{UserID: userID}) - unittest.AssertNotExistsBean(t, &models.Follow{FollowID: userID}) + unittest.AssertNotExistsBean(t, &user_model.Follow{UserID: userID}) + unittest.AssertNotExistsBean(t, &user_model.Follow{FollowID: userID}) unittest.AssertNotExistsBean(t, &models.Repository{OwnerID: userID}) unittest.AssertNotExistsBean(t, &models.Access{UserID: userID}) unittest.AssertNotExistsBean(t, &models.OrgUser{UID: userID}) diff --git a/models/error.go b/models/error.go index db0fce0ce1..7d9b2ae65b 100644 --- a/models/error.go +++ b/models/error.go @@ -155,21 +155,6 @@ func (err ErrUserInactive) Error() string { return fmt.Sprintf("user is inactive [uid: %d, name: %s]", err.UID, err.Name) } -// ErrOpenIDAlreadyUsed represents a "OpenIDAlreadyUsed" kind of error. -type ErrOpenIDAlreadyUsed struct { - OpenID string -} - -// IsErrOpenIDAlreadyUsed checks if an error is a ErrOpenIDAlreadyUsed. -func IsErrOpenIDAlreadyUsed(err error) bool { - _, ok := err.(ErrOpenIDAlreadyUsed) - return ok -} - -func (err ErrOpenIDAlreadyUsed) Error() string { - return fmt.Sprintf("OpenID already in use [oid: %s]", err.OpenID) -} - // ErrUserOwnRepos represents a "UserOwnRepos" kind of error. type ErrUserOwnRepos struct { UID int64 diff --git a/models/statistic.go b/models/statistic.go index fab35e62dc..1849497cd9 100644 --- a/models/statistic.go +++ b/models/statistic.go @@ -7,6 +7,7 @@ package models import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/login" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/modules/setting" ) @@ -92,7 +93,7 @@ func GetStatistic() (stats Statistic) { stats.Counter.Comment, _ = e.Count(new(Comment)) stats.Counter.Oauth = 0 - stats.Counter.Follow, _ = e.Count(new(Follow)) + stats.Counter.Follow, _ = e.Count(new(user_model.Follow)) stats.Counter.Mirror, _ = e.Count(new(Mirror)) stats.Counter.Release, _ = e.Count(new(Release)) stats.Counter.LoginSource = login.CountSources() diff --git a/models/user.go b/models/user.go index 8146c184e7..e3cf94efe0 100644 --- a/models/user.go +++ b/models/user.go @@ -25,6 +25,7 @@ import ( "code.gitea.io/gitea/models/login" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/auth/openid" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" @@ -359,7 +360,7 @@ func (u *User) GetFollowers(listOptions db.ListOptions) ([]*User, error) { // IsFollowing returns true if user is following followID. func (u *User) IsFollowing(followID int64) bool { - return IsFollowing(u.ID, followID) + return user_model.IsFollowing(u.ID, followID) } // GetFollowing returns range of user's following. @@ -469,7 +470,7 @@ func (u *User) isVisibleToUser(e db.Engine, viewer *User) bool { } // If they follow - they see each over - follower := IsFollowing(u.ID, viewer.ID) + follower := user_model.IsFollowing(u.ID, viewer.ID) if follower { return true } @@ -1212,12 +1213,12 @@ func deleteUser(e db.Engine, u *User) error { &Access{UserID: u.ID}, &Watch{UserID: u.ID}, &Star{UID: u.ID}, - &Follow{UserID: u.ID}, - &Follow{FollowID: u.ID}, + &user_model.Follow{UserID: u.ID}, + &user_model.Follow{FollowID: u.ID}, &Action{UserID: u.ID}, &IssueUser{UID: u.ID}, &user_model.EmailAddress{UID: u.ID}, - &UserOpenID{UID: u.ID}, + &user_model.UserOpenID{UID: u.ID}, &Reaction{UserID: u.ID}, &TeamUser{UID: u.ID}, &Collaboration{UserID: u.ID}, @@ -1798,3 +1799,29 @@ func IterateUser(f func(user *User) error) error { } } } + +// GetUserByOpenID returns the user object by given OpenID if exists. +func GetUserByOpenID(uri string) (*User, error) { + if len(uri) == 0 { + return nil, ErrUserNotExist{0, uri, 0} + } + + uri, err := openid.Normalize(uri) + if err != nil { + return nil, err + } + + log.Trace("Normalized OpenID URI: " + uri) + + // Otherwise, check in openid table + oid := &user_model.UserOpenID{} + has, err := db.GetEngine(db.DefaultContext).Where("uri=?", uri).Get(oid) + if err != nil { + return nil, err + } + if has { + return GetUserByID(oid.UID) + } + + return nil, ErrUserNotExist{0, uri, 0} +} diff --git a/models/user_follow.go b/models/user/follow.go similarity index 99% rename from models/user_follow.go rename to models/user/follow.go index 8832aa2f18..89675b5078 100644 --- a/models/user_follow.go +++ b/models/user/follow.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package models +package user import ( "code.gitea.io/gitea/models/db" diff --git a/models/user/follow_test.go b/models/user/follow_test.go new file mode 100644 index 0000000000..538c7b18a2 --- /dev/null +++ b/models/user/follow_test.go @@ -0,0 +1,22 @@ +// Copyright 2020 The Gitea 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 user + +import ( + "testing" + + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestIsFollowing(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + assert.True(t, IsFollowing(4, 2)) + assert.False(t, IsFollowing(2, 4)) + assert.False(t, IsFollowing(5, unittest.NonexistentID)) + assert.False(t, IsFollowing(unittest.NonexistentID, 5)) + assert.False(t, IsFollowing(unittest.NonexistentID, unittest.NonexistentID)) +} diff --git a/models/user/main_test.go b/models/user/main_test.go index 1dd9fb2781..7862556a03 100644 --- a/models/user/main_test.go +++ b/models/user/main_test.go @@ -15,5 +15,7 @@ func TestMain(m *testing.M) { unittest.MainTest(m, filepath.Join("..", ".."), "email_address.yml", "user_redirect.yml", + "follow.yml", + "user_open_id.yml", ) } diff --git a/models/user_openid.go b/models/user/openid.go similarity index 77% rename from models/user_openid.go rename to models/user/openid.go index 17a58536a2..8ca3c7f2c8 100644 --- a/models/user_openid.go +++ b/models/user/openid.go @@ -2,21 +2,21 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package models +package user import ( "errors" + "fmt" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/modules/auth/openid" - "code.gitea.io/gitea/modules/log" ) // ErrOpenIDNotExist openid is not known var ErrOpenIDNotExist = errors.New("OpenID is unknown") // UserOpenID is the list of all OpenID identities of a user. -type UserOpenID struct { +// Since this is a middle table, name it OpenID is not suitable, so we ignore the lint here +type UserOpenID struct { //revive:disable-line:exported ID int64 `xorm:"pk autoincr"` UID int64 `xorm:"INDEX NOT NULL"` URI string `xorm:"UNIQUE NOT NULL"` @@ -49,6 +49,21 @@ func isOpenIDUsed(e db.Engine, uri string) (bool, error) { return e.Get(&UserOpenID{URI: uri}) } +// ErrOpenIDAlreadyUsed represents a "OpenIDAlreadyUsed" kind of error. +type ErrOpenIDAlreadyUsed struct { + OpenID string +} + +// IsErrOpenIDAlreadyUsed checks if an error is a ErrOpenIDAlreadyUsed. +func IsErrOpenIDAlreadyUsed(err error) bool { + _, ok := err.(ErrOpenIDAlreadyUsed) + return ok +} + +func (err ErrOpenIDAlreadyUsed) Error() string { + return fmt.Sprintf("OpenID already in use [oid: %s]", err.OpenID) +} + // NOTE: make sure openid.URI is normalized already func addUserOpenID(e db.Engine, openid *UserOpenID) error { used, err := isOpenIDUsed(e, openid.URI) @@ -95,29 +110,3 @@ func ToggleUserOpenIDVisibility(id int64) (err error) { _, err = db.GetEngine(db.DefaultContext).Exec("update `user_open_id` set `show` = not `show` where `id` = ?", id) return err } - -// GetUserByOpenID returns the user object by given OpenID if exists. -func GetUserByOpenID(uri string) (*User, error) { - if len(uri) == 0 { - return nil, ErrUserNotExist{0, uri, 0} - } - - uri, err := openid.Normalize(uri) - if err != nil { - return nil, err - } - - log.Trace("Normalized OpenID URI: " + uri) - - // Otherwise, check in openid table - oid := &UserOpenID{} - has, err := db.GetEngine(db.DefaultContext).Where("uri=?", uri).Get(oid) - if err != nil { - return nil, err - } - if has { - return GetUserByID(oid.UID) - } - - return nil, ErrUserNotExist{0, uri, 0} -} diff --git a/models/user_openid_test.go b/models/user/openid_test.go similarity index 75% rename from models/user_openid_test.go rename to models/user/openid_test.go index d0d801ad18..ba678ef864 100644 --- a/models/user_openid_test.go +++ b/models/user/openid_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package models +package user import ( "testing" @@ -30,25 +30,6 @@ func TestGetUserOpenIDs(t *testing.T) { } } -func TestGetUserByOpenID(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - _, err := GetUserByOpenID("https://unknown") - if assert.Error(t, err) { - assert.True(t, IsErrUserNotExist(err)) - } - - user, err := GetUserByOpenID("https://user1.domain1.tld") - if assert.NoError(t, err) { - assert.Equal(t, int64(1), user.ID) - } - - user, err = GetUserByOpenID("https://domain1.tld/user2/") - if assert.NoError(t, err) { - assert.Equal(t, int64(2), user.ID) - } -} - func TestToggleUserOpenIDVisibility(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) oids, err := GetUserOpenIDs(int64(2)) diff --git a/models/user_follow_test.go b/models/user_follow_test.go deleted file mode 100644 index 5ba922728a..0000000000 --- a/models/user_follow_test.go +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2020 The Gitea 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 models - -import ( - "testing" - - "code.gitea.io/gitea/models/unittest" - "github.com/stretchr/testify/assert" -) - -func TestIsFollowing(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - assert.True(t, IsFollowing(4, 2)) - assert.False(t, IsFollowing(2, 4)) - assert.False(t, IsFollowing(5, unittest.NonexistentID)) - assert.False(t, IsFollowing(unittest.NonexistentID, 5)) - assert.False(t, IsFollowing(unittest.NonexistentID, unittest.NonexistentID)) -} - -func TestFollowUser(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - testSuccess := func(followerID, followedID int64) { - assert.NoError(t, FollowUser(followerID, followedID)) - unittest.AssertExistsAndLoadBean(t, &Follow{UserID: followerID, FollowID: followedID}) - } - testSuccess(4, 2) - testSuccess(5, 2) - - assert.NoError(t, FollowUser(2, 2)) - - unittest.CheckConsistencyFor(t, &User{}) -} - -func TestUnfollowUser(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - testSuccess := func(followerID, followedID int64) { - assert.NoError(t, UnfollowUser(followerID, followedID)) - unittest.AssertNotExistsBean(t, &Follow{UserID: followerID, FollowID: followedID}) - } - testSuccess(4, 2) - testSuccess(5, 2) - testSuccess(2, 2) - - unittest.CheckConsistencyFor(t, &User{}) -} diff --git a/models/user_test.go b/models/user_test.go index dc273ce1bc..4e2e521767 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -560,3 +560,51 @@ func TestNewUserRedirect3(t *testing.T) { RedirectUserID: user.ID, }) } + +func TestFollowUser(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + testSuccess := func(followerID, followedID int64) { + assert.NoError(t, user_model.FollowUser(followerID, followedID)) + unittest.AssertExistsAndLoadBean(t, &user_model.Follow{UserID: followerID, FollowID: followedID}) + } + testSuccess(4, 2) + testSuccess(5, 2) + + assert.NoError(t, user_model.FollowUser(2, 2)) + + unittest.CheckConsistencyFor(t, &User{}) +} + +func TestUnfollowUser(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + testSuccess := func(followerID, followedID int64) { + assert.NoError(t, user_model.UnfollowUser(followerID, followedID)) + unittest.AssertNotExistsBean(t, &user_model.Follow{UserID: followerID, FollowID: followedID}) + } + testSuccess(4, 2) + testSuccess(5, 2) + testSuccess(2, 2) + + unittest.CheckConsistencyFor(t, &User{}) +} + +func TestGetUserByOpenID(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + _, err := GetUserByOpenID("https://unknown") + if assert.Error(t, err) { + assert.True(t, IsErrUserNotExist(err)) + } + + user, err := GetUserByOpenID("https://user1.domain1.tld") + if assert.NoError(t, err) { + assert.Equal(t, int64(1), user.ID) + } + + user, err = GetUserByOpenID("https://domain1.tld/user2/") + if assert.NoError(t, err) { + assert.Equal(t, int64(2), user.ID) + } +} diff --git a/routers/api/v1/user/follower.go b/routers/api/v1/user/follower.go index e273ac6a02..5d66f3bcc3 100644 --- a/routers/api/v1/user/follower.go +++ b/routers/api/v1/user/follower.go @@ -9,6 +9,7 @@ import ( "net/http" "code.gitea.io/gitea/models" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/convert" api "code.gitea.io/gitea/modules/structs" @@ -239,7 +240,7 @@ func Follow(ctx *context.APIContext) { if ctx.Written() { return } - if err := models.FollowUser(ctx.User.ID, target.ID); err != nil { + if err := user_model.FollowUser(ctx.User.ID, target.ID); err != nil { ctx.Error(http.StatusInternalServerError, "FollowUser", err) return } @@ -265,7 +266,7 @@ func Unfollow(ctx *context.APIContext) { if ctx.Written() { return } - if err := models.UnfollowUser(ctx.User.ID, target.ID); err != nil { + if err := user_model.UnfollowUser(ctx.User.ID, target.ID); err != nil { ctx.Error(http.StatusInternalServerError, "UnfollowUser", err) return } diff --git a/routers/web/user/auth_openid.go b/routers/web/user/auth_openid.go index e6ad6fef4c..4724a7b431 100644 --- a/routers/web/user/auth_openid.go +++ b/routers/web/user/auth_openid.go @@ -10,6 +10,7 @@ import ( "net/url" "code.gitea.io/gitea/models" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/auth/openid" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" @@ -302,9 +303,9 @@ func ConnectOpenIDPost(ctx *context.Context) { } // add OpenID for the user - userOID := &models.UserOpenID{UID: u.ID, URI: oid} - if err = models.AddUserOpenID(userOID); err != nil { - if models.IsErrOpenIDAlreadyUsed(err) { + userOID := &user_model.UserOpenID{UID: u.ID, URI: oid} + if err = user_model.AddUserOpenID(userOID); err != nil { + if user_model.IsErrOpenIDAlreadyUsed(err) { ctx.RenderWithErr(ctx.Tr("form.openid_been_used", oid), tplConnectOID, &form) return } @@ -430,9 +431,9 @@ func RegisterOpenIDPost(ctx *context.Context) { } // add OpenID for the user - userOID := &models.UserOpenID{UID: u.ID, URI: oid} - if err = models.AddUserOpenID(userOID); err != nil { - if models.IsErrOpenIDAlreadyUsed(err) { + userOID := &user_model.UserOpenID{UID: u.ID, URI: oid} + if err = user_model.AddUserOpenID(userOID); err != nil { + if user_model.IsErrOpenIDAlreadyUsed(err) { ctx.RenderWithErr(ctx.Tr("form.openid_been_used", oid), tplSignUpOID, &form) return } diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 17c4783c69..72d36761da 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -131,7 +131,7 @@ func Profile(ctx *context.Context) { } // Show OpenID URIs - openIDs, err := models.GetUserOpenIDs(ctxUser.ID) + openIDs, err := user_model.GetUserOpenIDs(ctxUser.ID) if err != nil { ctx.ServerError("GetUserOpenIDs", err) return @@ -355,9 +355,9 @@ func Action(ctx *context.Context) { var err error switch ctx.Params(":action") { case "follow": - err = models.FollowUser(ctx.User.ID, u.ID) + err = user_model.FollowUser(ctx.User.ID, u.ID) case "unfollow": - err = models.UnfollowUser(ctx.User.ID, u.ID) + err = user_model.UnfollowUser(ctx.User.ID, u.ID) } if err != nil { diff --git a/routers/web/user/setting/security.go b/routers/web/user/setting/security.go index 65e9790d47..f0b1d8232a 100644 --- a/routers/web/user/setting/security.go +++ b/routers/web/user/setting/security.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/login" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/setting" @@ -107,7 +108,7 @@ func loadSecurityData(ctx *context.Context) { } ctx.Data["AccountLinks"] = sources - openid, err := models.GetUserOpenIDs(ctx.User.ID) + openid, err := user_model.GetUserOpenIDs(ctx.User.ID) if err != nil { ctx.ServerError("GetUserOpenIDs", err) return diff --git a/routers/web/user/setting/security_openid.go b/routers/web/user/setting/security_openid.go index 8bb932805c..9cdda79b92 100644 --- a/routers/web/user/setting/security_openid.go +++ b/routers/web/user/setting/security_openid.go @@ -7,7 +7,7 @@ package setting import ( "net/http" - "code.gitea.io/gitea/models" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/auth/openid" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" @@ -45,7 +45,7 @@ func OpenIDPost(ctx *context.Context) { form.Openid = id log.Trace("Normalized id: " + id) - oids, err := models.GetUserOpenIDs(ctx.User.ID) + oids, err := user_model.GetUserOpenIDs(ctx.User.ID) if err != nil { ctx.ServerError("GetUserOpenIDs", err) return @@ -89,9 +89,9 @@ func settingsOpenIDVerify(ctx *context.Context) { log.Trace("Verified ID: " + id) - oid := &models.UserOpenID{UID: ctx.User.ID, URI: id} - if err = models.AddUserOpenID(oid); err != nil { - if models.IsErrOpenIDAlreadyUsed(err) { + oid := &user_model.UserOpenID{UID: ctx.User.ID, URI: id} + if err = user_model.AddUserOpenID(oid); err != nil { + if user_model.IsErrOpenIDAlreadyUsed(err) { ctx.RenderWithErr(ctx.Tr("form.openid_been_used", id), tplSettingsSecurity, &forms.AddOpenIDForm{Openid: id}) return } @@ -106,7 +106,7 @@ func settingsOpenIDVerify(ctx *context.Context) { // DeleteOpenID response for delete user's openid func DeleteOpenID(ctx *context.Context) { - if err := models.DeleteUserOpenID(&models.UserOpenID{ID: ctx.FormInt64("id"), UID: ctx.User.ID}); err != nil { + if err := user_model.DeleteUserOpenID(&user_model.UserOpenID{ID: ctx.FormInt64("id"), UID: ctx.User.ID}); err != nil { ctx.ServerError("DeleteUserOpenID", err) return } @@ -120,7 +120,7 @@ func DeleteOpenID(ctx *context.Context) { // ToggleOpenIDVisibility response for toggle visibility of user's openid func ToggleOpenIDVisibility(ctx *context.Context) { - if err := models.ToggleUserOpenIDVisibility(ctx.FormInt64("id")); err != nil { + if err := user_model.ToggleUserOpenIDVisibility(ctx.FormInt64("id")); err != nil { ctx.ServerError("ToggleUserOpenIDVisibility", err) return }