From cf02cd7ba0c94165743660cf30f0cbb5a73a385e Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Sat, 20 May 2017 04:48:22 -0400 Subject: Fix and test for delete user (#1713) * Fix and test for delete user * Run updates in batches * Unit test --- integrations/delete_user_test.go | 41 ++++++ models/consistency.go | 172 ++++++++++++++++++++++++ models/consistency_test.go | 172 ------------------------ models/fixtures/follow.yml | 10 ++ models/fixtures/user.yml | 9 +- models/main_test.go | 26 ++++ models/setup_for_test.go | 111 --------------- models/unit_tests.go | 92 +++++++++++++ models/user.go | 44 +++--- models/user_test.go | 31 +++++ vendor/github.com/go-xorm/builder/cond_in.go | 6 +- vendor/github.com/go-xorm/builder/cond_notin.go | 6 +- vendor/vendor.json | 6 +- 13 files changed, 409 insertions(+), 317 deletions(-) create mode 100644 integrations/delete_user_test.go create mode 100644 models/consistency.go delete mode 100644 models/consistency_test.go create mode 100644 models/main_test.go delete mode 100644 models/setup_for_test.go create mode 100644 models/unit_tests.go diff --git a/integrations/delete_user_test.go b/integrations/delete_user_test.go new file mode 100644 index 0000000000..bf38eeae2b --- /dev/null +++ b/integrations/delete_user_test.go @@ -0,0 +1,41 @@ +// Copyright 2017 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 integrations + +import ( + "bytes" + "net/http" + "net/url" + "testing" + + "code.gitea.io/gitea/models" + + "github.com/stretchr/testify/assert" +) + +func TestDeleteUser(t *testing.T) { + prepareTestEnv(t) + + session := loginUser(t, "user1", "password") + + req, err := http.NewRequest("GET", "/admin/users/8", nil) + assert.NoError(t, err) + resp := session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + + doc, err := NewHtmlParser(resp.Body) + assert.NoError(t, err) + req, err = http.NewRequest("POST", "/admin/users/8/delete", + bytes.NewBufferString(url.Values{ + "_csrf": []string{doc.GetInputValueByName("_csrf")}, + }.Encode())) + assert.NoError(t, err) + req.Header.Add("Content-Type", "application/x-www-form-urlencoded") + resp = session.MakeRequest(t, req) + assert.EqualValues(t, http.StatusOK, resp.HeaderCode) + + models.AssertNotExistsBean(t, &models.User{ID: 8}) + models.CheckConsistencyFor(t, &models.User{}) +} diff --git a/models/consistency.go b/models/consistency.go new file mode 100644 index 0000000000..77308e538e --- /dev/null +++ b/models/consistency.go @@ -0,0 +1,172 @@ +// Copyright 2017 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 ( + "reflect" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// consistencyCheckable a type that can be tested for database consistency +type consistencyCheckable interface { + checkForConsistency(t *testing.T) +} + +// CheckConsistencyForAll test that the entire database is consistent +func CheckConsistencyForAll(t *testing.T) { + CheckConsistencyFor(t, + &User{}, + &Repository{}, + &Issue{}, + &PullRequest{}, + &Milestone{}, + &Label{}, + &Team{}, + &Action{}) +} + +// CheckConsistencyFor test that all matching database entries are consistent +func CheckConsistencyFor(t *testing.T, beansToCheck ...interface{}) { + for _, bean := range beansToCheck { + sliceType := reflect.SliceOf(reflect.TypeOf(bean)) + sliceValue := reflect.MakeSlice(sliceType, 0, 10) + + ptrToSliceValue := reflect.New(sliceType) + ptrToSliceValue.Elem().Set(sliceValue) + + assert.NoError(t, x.Where(bean).Find(ptrToSliceValue.Interface())) + sliceValue = ptrToSliceValue.Elem() + + for i := 0; i < sliceValue.Len(); i++ { + entity := sliceValue.Index(i).Interface() + checkable, ok := entity.(consistencyCheckable) + if !ok { + t.Errorf("Expected %+v (of type %T) to be checkable for consistency", + entity, entity) + } else { + checkable.checkForConsistency(t) + } + } + } +} + +// getCount get the count of database entries matching bean +func getCount(t *testing.T, e Engine, bean interface{}) int64 { + count, err := e.Count(bean) + assert.NoError(t, err) + return count +} + +// assertCount test the count of database entries matching bean +func assertCount(t *testing.T, bean interface{}, expected int) { + assert.EqualValues(t, expected, getCount(t, x, bean), + "Failed consistency test, the counted bean (of type %T) was %+v", bean, bean) +} + +func (user *User) checkForConsistency(t *testing.T) { + assertCount(t, &Repository{OwnerID: user.ID}, user.NumRepos) + assertCount(t, &Star{UID: user.ID}, user.NumStars) + assertCount(t, &OrgUser{OrgID: user.ID}, user.NumMembers) + assertCount(t, &Team{OrgID: user.ID}, user.NumTeams) + assertCount(t, &Follow{UserID: user.ID}, user.NumFollowing) + assertCount(t, &Follow{FollowID: user.ID}, user.NumFollowers) + if user.Type != UserTypeOrganization { + assert.EqualValues(t, 0, user.NumMembers) + assert.EqualValues(t, 0, user.NumTeams) + } +} + +func (repo *Repository) checkForConsistency(t *testing.T) { + assert.Equal(t, repo.LowerName, strings.ToLower(repo.Name), "repo: %+v", repo) + assertCount(t, &Star{RepoID: repo.ID}, repo.NumStars) + assertCount(t, &Watch{RepoID: repo.ID}, repo.NumWatches) + assertCount(t, &Milestone{RepoID: repo.ID}, repo.NumMilestones) + assertCount(t, &Repository{ForkID: repo.ID}, repo.NumForks) + if repo.IsFork { + AssertExistsAndLoadBean(t, &Repository{ID: repo.ForkID}) + } + + actual := getCount(t, x.Where("is_pull=?", false), &Issue{RepoID: repo.ID}) + assert.EqualValues(t, repo.NumIssues, actual, + "Unexpected number of issues for repo %+v", repo) + + actual = getCount(t, x.Where("is_pull=? AND is_closed=?", false, true), &Issue{RepoID: repo.ID}) + assert.EqualValues(t, repo.NumClosedIssues, actual, + "Unexpected number of closed issues for repo %+v", repo) + + actual = getCount(t, x.Where("is_pull=?", true), &Issue{RepoID: repo.ID}) + assert.EqualValues(t, repo.NumPulls, actual, + "Unexpected number of pulls for repo %+v", repo) + + actual = getCount(t, x.Where("is_pull=? AND is_closed=?", true, true), &Issue{RepoID: repo.ID}) + assert.EqualValues(t, repo.NumClosedPulls, actual, + "Unexpected number of closed pulls for repo %+v", repo) + + actual = getCount(t, x.Where("is_closed=?", true), &Milestone{RepoID: repo.ID}) + assert.EqualValues(t, repo.NumClosedMilestones, actual, + "Unexpected number of closed milestones for repo %+v", repo) +} + +func (issue *Issue) checkForConsistency(t *testing.T) { + actual := getCount(t, x.Where("type=?", CommentTypeComment), &Comment{IssueID: issue.ID}) + assert.EqualValues(t, issue.NumComments, actual, + "Unexpected number of comments for issue %+v", issue) + if issue.IsPull { + pr := AssertExistsAndLoadBean(t, &PullRequest{IssueID: issue.ID}).(*PullRequest) + assert.EqualValues(t, pr.Index, issue.Index) + } +} + +func (pr *PullRequest) checkForConsistency(t *testing.T) { + issue := AssertExistsAndLoadBean(t, &Issue{ID: pr.IssueID}).(*Issue) + assert.True(t, issue.IsPull) + assert.EqualValues(t, issue.Index, pr.Index) +} + +func (milestone *Milestone) checkForConsistency(t *testing.T) { + assertCount(t, &Issue{MilestoneID: milestone.ID}, milestone.NumIssues) + + actual := getCount(t, x.Where("is_closed=?", true), &Issue{MilestoneID: milestone.ID}) + assert.EqualValues(t, milestone.NumClosedIssues, actual, + "Unexpected number of closed issues for milestone %+v", milestone) +} + +func (label *Label) checkForConsistency(t *testing.T) { + issueLabels := make([]*IssueLabel, 0, 10) + assert.NoError(t, x.Find(&issueLabels, &IssueLabel{LabelID: label.ID})) + assert.EqualValues(t, label.NumIssues, len(issueLabels), + "Unexpected number of issue for label %+v", label) + + issueIDs := make([]int64, len(issueLabels)) + for i, issueLabel := range issueLabels { + issueIDs[i] = issueLabel.IssueID + } + + expected := int64(0) + if len(issueIDs) > 0 { + expected = getCount(t, x.In("id", issueIDs).Where("is_closed=?", true), &Issue{}) + } + assert.EqualValues(t, expected, label.NumClosedIssues, + "Unexpected number of closed issues for label %+v", label) +} + +func (team *Team) checkForConsistency(t *testing.T) { + assertCount(t, &TeamUser{TeamID: team.ID}, team.NumMembers) + assertCount(t, &TeamRepo{TeamID: team.ID}, team.NumRepos) +} + +func (action *Action) checkForConsistency(t *testing.T) { + repo := AssertExistsAndLoadBean(t, &Repository{ID: action.RepoID}).(*Repository) + owner := AssertExistsAndLoadBean(t, &User{ID: repo.OwnerID}).(*User) + actor := AssertExistsAndLoadBean(t, &User{ID: action.ActUserID}).(*User) + + assert.Equal(t, repo.Name, action.RepoName, "action: %+v", action) + assert.Equal(t, repo.IsPrivate, action.IsPrivate, "action: %+v", action) + assert.Equal(t, owner.Name, action.RepoUserName, "action: %+v", action) + assert.Equal(t, actor.Name, action.ActUserName, "action: %+v", action) +} diff --git a/models/consistency_test.go b/models/consistency_test.go deleted file mode 100644 index a8de8e3c2b..0000000000 --- a/models/consistency_test.go +++ /dev/null @@ -1,172 +0,0 @@ -// Copyright 2017 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 ( - "reflect" - "strings" - "testing" - - "github.com/stretchr/testify/assert" -) - -// ConsistencyCheckable a type that can be tested for database consistency -type ConsistencyCheckable interface { - CheckForConsistency(t *testing.T) -} - -// CheckConsistencyForAll test that the entire database is consistent -func CheckConsistencyForAll(t *testing.T) { - CheckConsistencyFor(t, - &User{}, - &Repository{}, - &Issue{}, - &PullRequest{}, - &Milestone{}, - &Label{}, - &Team{}, - &Action{}) -} - -// CheckConsistencyFor test that all matching database entries are consistent -func CheckConsistencyFor(t *testing.T, beansToCheck ...interface{}) { - for _, bean := range beansToCheck { - sliceType := reflect.SliceOf(reflect.TypeOf(bean)) - sliceValue := reflect.MakeSlice(sliceType, 0, 10) - - ptrToSliceValue := reflect.New(sliceType) - ptrToSliceValue.Elem().Set(sliceValue) - - assert.NoError(t, x.Where(bean).Find(ptrToSliceValue.Interface())) - sliceValue = ptrToSliceValue.Elem() - - for i := 0; i < sliceValue.Len(); i++ { - entity := sliceValue.Index(i).Interface() - checkable, ok := entity.(ConsistencyCheckable) - if !ok { - t.Errorf("Expected %+v (of type %T) to be checkable for consistency", - entity, entity) - } else { - checkable.CheckForConsistency(t) - } - } - } -} - -// getCount get the count of database entries matching bean -func getCount(t *testing.T, e Engine, bean interface{}) int64 { - count, err := e.Count(bean) - assert.NoError(t, err) - return count -} - -// assertCount test the count of database entries matching bean -func assertCount(t *testing.T, bean interface{}, expected int) { - assert.EqualValues(t, expected, getCount(t, x, bean), - "Failed consistency test, the counted bean (of type %T) was %+v", bean, bean) -} - -func (user *User) CheckForConsistency(t *testing.T) { - assertCount(t, &Repository{OwnerID: user.ID}, user.NumRepos) - assertCount(t, &Star{UID: user.ID}, user.NumStars) - assertCount(t, &OrgUser{OrgID: user.ID}, user.NumMembers) - assertCount(t, &Team{OrgID: user.ID}, user.NumTeams) - assertCount(t, &Follow{UserID: user.ID}, user.NumFollowing) - assertCount(t, &Follow{FollowID: user.ID}, user.NumFollowers) - if user.Type != UserTypeOrganization { - assert.EqualValues(t, 0, user.NumMembers) - assert.EqualValues(t, 0, user.NumTeams) - } -} - -func (repo *Repository) CheckForConsistency(t *testing.T) { - assert.Equal(t, repo.LowerName, strings.ToLower(repo.Name), "repo: %+v", repo) - assertCount(t, &Star{RepoID: repo.ID}, repo.NumStars) - assertCount(t, &Watch{RepoID: repo.ID}, repo.NumWatches) - assertCount(t, &Milestone{RepoID: repo.ID}, repo.NumMilestones) - assertCount(t, &Repository{ForkID: repo.ID}, repo.NumForks) - if repo.IsFork { - AssertExistsAndLoadBean(t, &Repository{ID: repo.ForkID}) - } - - actual := getCount(t, x.Where("is_pull=?", false), &Issue{RepoID: repo.ID}) - assert.EqualValues(t, repo.NumIssues, actual, - "Unexpected number of issues for repo %+v", repo) - - actual = getCount(t, x.Where("is_pull=? AND is_closed=?", false, true), &Issue{RepoID: repo.ID}) - assert.EqualValues(t, repo.NumClosedIssues, actual, - "Unexpected number of closed issues for repo %+v", repo) - - actual = getCount(t, x.Where("is_pull=?", true), &Issue{RepoID: repo.ID}) - assert.EqualValues(t, repo.NumPulls, actual, - "Unexpected number of pulls for repo %+v", repo) - - actual = getCount(t, x.Where("is_pull=? AND is_closed=?", true, true), &Issue{RepoID: repo.ID}) - assert.EqualValues(t, repo.NumClosedPulls, actual, - "Unexpected number of closed pulls for repo %+v", repo) - - actual = getCount(t, x.Where("is_closed=?", true), &Milestone{RepoID: repo.ID}) - assert.EqualValues(t, repo.NumClosedMilestones, actual, - "Unexpected number of closed milestones for repo %+v", repo) -} - -func (issue *Issue) CheckForConsistency(t *testing.T) { - actual := getCount(t, x.Where("type=?", CommentTypeComment), &Comment{IssueID: issue.ID}) - assert.EqualValues(t, issue.NumComments, actual, - "Unexpected number of comments for issue %+v", issue) - if issue.IsPull { - pr := AssertExistsAndLoadBean(t, &PullRequest{IssueID: issue.ID}).(*PullRequest) - assert.EqualValues(t, pr.Index, issue.Index) - } -} - -func (pr *PullRequest) CheckForConsistency(t *testing.T) { - issue := AssertExistsAndLoadBean(t, &Issue{ID: pr.IssueID}).(*Issue) - assert.True(t, issue.IsPull) - assert.EqualValues(t, issue.Index, pr.Index) -} - -func (milestone *Milestone) CheckForConsistency(t *testing.T) { - assertCount(t, &Issue{MilestoneID: milestone.ID}, milestone.NumIssues) - - actual := getCount(t, x.Where("is_closed=?", true), &Issue{MilestoneID: milestone.ID}) - assert.EqualValues(t, milestone.NumClosedIssues, actual, - "Unexpected number of closed issues for milestone %+v", milestone) -} - -func (label *Label) CheckForConsistency(t *testing.T) { - issueLabels := make([]*IssueLabel, 0, 10) - assert.NoError(t, x.Find(&issueLabels, &IssueLabel{LabelID: label.ID})) - assert.EqualValues(t, label.NumIssues, len(issueLabels), - "Unexpected number of issue for label %+v", label) - - issueIDs := make([]int64, len(issueLabels)) - for i, issueLabel := range issueLabels { - issueIDs[i] = issueLabel.IssueID - } - - expected := int64(0) - if len(issueIDs) > 0 { - expected = getCount(t, x.In("id", issueIDs).Where("is_closed=?", true), &Issue{}) - } - assert.EqualValues(t, expected, label.NumClosedIssues, - "Unexpected number of closed issues for label %+v", label) -} - -func (team *Team) CheckForConsistency(t *testing.T) { - assertCount(t, &TeamUser{TeamID: team.ID}, team.NumMembers) - assertCount(t, &TeamRepo{TeamID: team.ID}, team.NumRepos) -} - -func (action *Action) CheckForConsistency(t *testing.T) { - repo := AssertExistsAndLoadBean(t, &Repository{ID: action.RepoID}).(*Repository) - owner := AssertExistsAndLoadBean(t, &User{ID: repo.OwnerID}).(*User) - actor := AssertExistsAndLoadBean(t, &User{ID: action.ActUserID}).(*User) - - assert.Equal(t, repo.Name, action.RepoName, "action: %+v", action) - assert.Equal(t, repo.IsPrivate, action.IsPrivate, "action: %+v", action) - assert.Equal(t, owner.Name, action.RepoUserName, "action: %+v", action) - assert.Equal(t, actor.Name, action.ActUserName, "action: %+v", action) -} diff --git a/models/fixtures/follow.yml b/models/fixtures/follow.yml index 53db1e88b1..480fa065c7 100644 --- a/models/fixtures/follow.yml +++ b/models/fixtures/follow.yml @@ -2,3 +2,13 @@ id: 1 user_id: 4 follow_id: 2 + +- + id: 2 + user_id: 8 + follow_id: 2 + +- + id: 3 + user_id: 2 + follow_id: 8 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index dd8554c58b..1777cff77e 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -4,9 +4,9 @@ name: user1 full_name: User One email: user1@example.com - passwd: password + passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password type: 0 # individual - salt: salt + salt: ZogKvWdyEx is_admin: true avatar: avatar1 avatar_email: user1@example.com @@ -26,7 +26,8 @@ avatar_email: user2@example.com num_repos: 2 num_stars: 2 - num_followers: 1 + num_followers: 2 + num_following: 1 is_active: true - @@ -123,6 +124,8 @@ avatar_email: user8@example.com num_repos: 0 is_active: true + num_followers: 1 + num_following: 1 - id: 9 diff --git a/models/main_test.go b/models/main_test.go new file mode 100644 index 0000000000..304bbccaa9 --- /dev/null +++ b/models/main_test.go @@ -0,0 +1,26 @@ +package models + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "code.gitea.io/gitea/modules/setting" +) + +func TestMain(m *testing.M) { + if err := CreateTestEngine(); err != nil { + fmt.Printf("Error creating test engine: %v\n", err) + os.Exit(1) + } + + setting.AppURL = "https://try.gitea.io/" + setting.RunUser = "runuser" + setting.SSH.Port = 3000 + setting.SSH.Domain = "try.gitea.io" + setting.RepoRootPath = filepath.Join(os.TempDir(), "repos") + setting.AppDataPath = filepath.Join(os.TempDir(), "appdata") + + os.Exit(m.Run()) +} diff --git a/models/setup_for_test.go b/models/setup_for_test.go deleted file mode 100644 index 9fb83f15c4..0000000000 --- a/models/setup_for_test.go +++ /dev/null @@ -1,111 +0,0 @@ -// Copyright 2016 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 ( - "fmt" - "os" - "path/filepath" - "testing" - - "code.gitea.io/gitea/modules/setting" - - "github.com/go-xorm/core" - "github.com/go-xorm/xorm" - _ "github.com/mattn/go-sqlite3" // for the test engine - "github.com/stretchr/testify/assert" - "gopkg.in/testfixtures.v2" -) - -const NonexistentID = 9223372036854775807 - -func TestMain(m *testing.M) { - if err := CreateTestEngine(); err != nil { - fmt.Printf("Error creating test engine: %v\n", err) - os.Exit(1) - } - - setting.AppURL = "https://try.gitea.io/" - setting.RunUser = "runuser" - setting.SSH.Port = 3000 - setting.SSH.Domain = "try.gitea.io" - setting.RepoRootPath = filepath.Join(os.TempDir(), "repos") - setting.AppDataPath = filepath.Join(os.TempDir(), "appdata") - - os.Exit(m.Run()) -} - -// CreateTestEngine create an xorm engine for testing -func CreateTestEngine() error { - var err error - x, err = xorm.NewEngine("sqlite3", "file::memory:?cache=shared") - if err != nil { - return err - } - x.SetMapper(core.GonicMapper{}) - if err = x.StoreEngine("InnoDB").Sync2(tables...); err != nil { - return err - } - - return InitFixtures(&testfixtures.SQLite{}, "fixtures/") -} - -func TestFixturesAreConsistent(t *testing.T) { - assert.NoError(t, PrepareTestDatabase()) - CheckConsistencyForAll(t) -} - -// PrepareTestDatabase load test fixtures into test database -func PrepareTestDatabase() error { - return LoadFixtures() -} - -func loadBeanIfExists(bean interface{}, conditions ...interface{}) (bool, error) { - sess := x.NewSession() - defer sess.Close() - - for _, cond := range conditions { - sess = sess.Where(cond) - } - return sess.Get(bean) -} - -// BeanExists for testing, check if a bean exists -func BeanExists(t *testing.T, bean interface{}, conditions ...interface{}) bool { - exists, err := loadBeanIfExists(bean, conditions...) - assert.NoError(t, err) - return exists -} - -// AssertExistsAndLoadBean assert that a bean exists and load it from the test -// database -func AssertExistsAndLoadBean(t *testing.T, bean interface{}, conditions ...interface{}) interface{} { - exists, err := loadBeanIfExists(bean, conditions...) - assert.NoError(t, err) - assert.True(t, exists, - "Expected to find %+v (of type %T, with conditions %+v), but did not", - bean, bean, conditions) - return bean -} - -// AssertNotExistsBean assert that a bean does not exist in the test database -func AssertNotExistsBean(t *testing.T, bean interface{}, conditions ...interface{}) { - exists, err := loadBeanIfExists(bean, conditions...) - assert.NoError(t, err) - assert.False(t, exists) -} - -// AssertSuccessfulInsert assert that beans is successfully inserted -func AssertSuccessfulInsert(t *testing.T, beans ...interface{}) { - _, err := x.Insert(beans...) - assert.NoError(t, err) -} - -// AssertCount assert the count of a bean -func AssertCount(t *testing.T, bean interface{}, expected interface{}) { - actual, err := x.Count(bean) - assert.NoError(t, err) - assert.EqualValues(t, expected, actual) -} diff --git a/models/unit_tests.go b/models/unit_tests.go new file mode 100644 index 0000000000..44b2e2c637 --- /dev/null +++ b/models/unit_tests.go @@ -0,0 +1,92 @@ +// Copyright 2016 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" + + "github.com/go-xorm/core" + "github.com/go-xorm/xorm" + _ "github.com/mattn/go-sqlite3" // for the test engine + "github.com/stretchr/testify/assert" + "gopkg.in/testfixtures.v2" +) + +// NonexistentID an ID that will never exist +const NonexistentID = 9223372036854775807 + +// CreateTestEngine create an xorm engine for testing +func CreateTestEngine() error { + var err error + x, err = xorm.NewEngine("sqlite3", "file::memory:?cache=shared") + if err != nil { + return err + } + x.SetMapper(core.GonicMapper{}) + if err = x.StoreEngine("InnoDB").Sync2(tables...); err != nil { + return err + } + + return InitFixtures(&testfixtures.SQLite{}, "fixtures/") +} + +// TestFixturesAreConsistent assert that test fixtures are consistent +func TestFixturesAreConsistent(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + CheckConsistencyForAll(t) +} + +// PrepareTestDatabase load test fixtures into test database +func PrepareTestDatabase() error { + return LoadFixtures() +} + +func loadBeanIfExists(bean interface{}, conditions ...interface{}) (bool, error) { + sess := x.NewSession() + defer sess.Close() + + for _, cond := range conditions { + sess = sess.Where(cond) + } + return sess.Get(bean) +} + +// BeanExists for testing, check if a bean exists +func BeanExists(t *testing.T, bean interface{}, conditions ...interface{}) bool { + exists, err := loadBeanIfExists(bean, conditions...) + assert.NoError(t, err) + return exists +} + +// AssertExistsAndLoadBean assert that a bean exists and load it from the test +// database +func AssertExistsAndLoadBean(t *testing.T, bean interface{}, conditions ...interface{}) interface{} { + exists, err := loadBeanIfExists(bean, conditions...) + assert.NoError(t, err) + assert.True(t, exists, + "Expected to find %+v (of type %T, with conditions %+v), but did not", + bean, bean, conditions) + return bean +} + +// AssertNotExistsBean assert that a bean does not exist in the test database +func AssertNotExistsBean(t *testing.T, bean interface{}, conditions ...interface{}) { + exists, err := loadBeanIfExists(bean, conditions...) + assert.NoError(t, err) + assert.False(t, exists) +} + +// AssertSuccessfulInsert assert that beans is successfully inserted +func AssertSuccessfulInsert(t *testing.T, beans ...interface{}) { + _, err := x.Insert(beans...) + assert.NoError(t, err) +} + +// AssertCount assert the count of a bean +func AssertCount(t *testing.T, bean interface{}, expected interface{}) { + actual, err := x.Count(bean) + assert.NoError(t, err) + assert.EqualValues(t, expected, actual) +} diff --git a/models/user.go b/models/user.go index 75329d8ff0..5fe72f923a 100644 --- a/models/user.go +++ b/models/user.go @@ -924,38 +924,41 @@ func deleteUser(e *xorm.Session, u *User) error { } // ***** START: Watch ***** - watches := make([]*Watch, 0, 10) - if err = e.Find(&watches, &Watch{UserID: u.ID}); err != nil { + watchedRepoIDs := make([]int64, 0, 10) + if err = e.Table("watch").Cols("watch.repo_id"). + Where("watch.user_id = ?", u.ID).Find(&watchedRepoIDs); err != nil { return fmt.Errorf("get all watches: %v", err) } - for i := range watches { - if _, err = e.Exec("UPDATE `repository` SET num_watches=num_watches-1 WHERE id=?", watches[i].RepoID); err != nil { - return fmt.Errorf("decrease repository watch number[%d]: %v", watches[i].RepoID, err) - } + if _, err = e.Decr("num_watches").In("id", watchedRepoIDs).Update(new(Repository)); err != nil { + return fmt.Errorf("decrease repository num_watches: %v", err) } // ***** END: Watch ***** // ***** START: Star ***** - stars := make([]*Star, 0, 10) - if err = e.Find(&stars, &Star{UID: u.ID}); err != nil { + starredRepoIDs := make([]int64, 0, 10) + if err = e.Table("star").Cols("star.repo_id"). + Where("star.uid = ?", u.ID).Find(&starredRepoIDs); err != nil { return fmt.Errorf("get all stars: %v", err) - } - for i := range stars { - if _, err = e.Exec("UPDATE `repository` SET num_stars=num_stars-1 WHERE id=?", stars[i].RepoID); err != nil { - return fmt.Errorf("decrease repository star number[%d]: %v", stars[i].RepoID, err) - } + } else if _, err = e.Decr("num_watches").In("id", starredRepoIDs).Update(new(Repository)); err != nil { + return fmt.Errorf("decrease repository num_stars: %v", err) } // ***** END: Star ***** // ***** START: Follow ***** - followers := make([]*Follow, 0, 10) - if err = e.Find(&followers, &Follow{UserID: u.ID}); err != nil { - return fmt.Errorf("get all followers: %v", err) + followeeIDs := make([]int64, 0, 10) + if err = e.Table("follow").Cols("follow.follow_id"). + Where("follow.user_id = ?", u.ID).Find(&followeeIDs); err != nil { + return fmt.Errorf("get all followees: %v", err) + } else if _, err = e.Decr("num_followers").In("id", followeeIDs).Update(new(User)); err != nil { + return fmt.Errorf("decrease user num_followers: %v", err) } - for i := range followers { - if _, err = e.Exec("UPDATE `user` SET num_followers=num_followers-1 WHERE id=?", followers[i].UserID); err != nil { - return fmt.Errorf("decrease user follower number[%d]: %v", followers[i].UserID, err) - } + + followerIDs := make([]int64, 0, 10) + if err = e.Table("follow").Cols("follow.user_id"). + Where("follow.follow_id = ?", u.ID).Find(&followerIDs); err != nil { + return fmt.Errorf("get all followers: %v", err) + } else if _, err = e.Decr("num_following").In("id", followerIDs).Update(new(User)); err != nil { + return fmt.Errorf("decrease user num_following: %v", err) } // ***** END: Follow ***** @@ -965,6 +968,7 @@ func deleteUser(e *xorm.Session, u *User) error { &Access{UserID: u.ID}, &Watch{UserID: u.ID}, &Star{UID: u.ID}, + &Follow{UserID: u.ID}, &Follow{FollowID: u.ID}, &Action{UserID: u.ID}, &IssueUser{UID: u.ID}, diff --git a/models/user_test.go b/models/user_test.go index b10ed9dcba..a16f979530 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -36,5 +36,36 @@ func TestCanCreateOrganization(t *testing.T) { user.AllowCreateOrganization = true assert.True(t, admin.CanCreateOrganization()) assert.False(t, user.CanCreateOrganization()) +} + +func TestDeleteUser(t *testing.T) { + test := func(userID int64) { + assert.NoError(t, PrepareTestDatabase()) + user := AssertExistsAndLoadBean(t, &User{ID: userID}).(*User) + + ownedRepos := make([]*Repository, 0, 10) + assert.NoError(t, x.Find(&ownedRepos, &Repository{OwnerID: userID})) + if len(ownedRepos) > 0 { + err := DeleteUser(user) + assert.Error(t, err) + assert.True(t, IsErrUserOwnRepos(err)) + return + } + orgUsers := make([]*OrgUser, 0, 10) + assert.NoError(t, x.Find(&orgUsers, &OrgUser{UID: userID})) + for _, orgUser := range orgUsers { + if err := RemoveOrgUser(orgUser.OrgID, orgUser.UID); err != nil { + assert.True(t, IsErrLastOrgOwner(err)) + return + } + } + assert.NoError(t, DeleteUser(user)) + AssertNotExistsBean(t, &User{ID: userID}) + CheckConsistencyFor(t, &User{}, &Repository{}) + } + test(2) + test(4) + test(8) + test(11) } diff --git a/vendor/github.com/go-xorm/builder/cond_in.go b/vendor/github.com/go-xorm/builder/cond_in.go index 71093e4b49..f6366d35c2 100644 --- a/vendor/github.com/go-xorm/builder/cond_in.go +++ b/vendor/github.com/go-xorm/builder/cond_in.go @@ -23,10 +23,8 @@ func In(col string, values ...interface{}) Cond { } func (condIn condIn) handleBlank(w Writer) error { - if _, err := fmt.Fprintf(w, "%s IN ()", condIn.col); err != nil { - return err - } - return nil + _, err := fmt.Fprint(w, "0=1") + return err } func (condIn condIn) WriteTo(w Writer) error { diff --git a/vendor/github.com/go-xorm/builder/cond_notin.go b/vendor/github.com/go-xorm/builder/cond_notin.go index 9be44cb2af..dc3ac49a35 100644 --- a/vendor/github.com/go-xorm/builder/cond_notin.go +++ b/vendor/github.com/go-xorm/builder/cond_notin.go @@ -20,10 +20,8 @@ func NotIn(col string, values ...interface{}) Cond { } func (condNotIn condNotIn) handleBlank(w Writer) error { - if _, err := fmt.Fprintf(w, "%s NOT IN ()", condNotIn.col); err != nil { - return err - } - return nil + _, err := fmt.Fprint(w, "0=0") + return err } func (condNotIn condNotIn) WriteTo(w Writer) error { diff --git a/vendor/vendor.json b/vendor/vendor.json index 93e8fa9d8b..109f67b998 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -450,10 +450,10 @@ "revisionTime": "2016-11-01T11:13:14Z" }, { - "checksumSHA1": "HHB+Jna1wv0cXLxtCyOnQqFwvn4=", + "checksumSHA1": "9SXbj96wb1PgppBZzxMIN0axbFQ=", "path": "github.com/go-xorm/builder", - "revision": "c6e604e9c7b7461715091e14ad0c242ec44c26e4", - "revisionTime": "2017-02-24T04:30:50Z" + "revision": "043186300e9b2c22abdfc83567a979e3af04d9ae", + "revisionTime": "2017-05-18T21:58:56Z" }, { "checksumSHA1": "vt2CGANHLNXPAZ01ve3UlsgQ0uU=", -- cgit v1.2.3