Fix and test for delete user (#1713)
* Fix and test for delete user * Run updates in batches * Unit test
This commit is contained in:
		
							parent
							
								
									85a7396525
								
							
						
					
					
						commit
						cf02cd7ba0
					
				
							
								
								
									
										41
									
								
								integrations/delete_user_test.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										41
									
								
								integrations/delete_user_test.go
									
									
									
									
									
										Normal file
									
								
							| @ -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{}) | ||||
| } | ||||
| @ -12,9 +12,9 @@ import ( | ||||
| 	"github.com/stretchr/testify/assert" | ||||
| ) | ||||
| 
 | ||||
| // ConsistencyCheckable a type that can be tested for database consistency
 | ||||
| type ConsistencyCheckable interface { | ||||
| 	CheckForConsistency(t *testing.T) | ||||
| // 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
 | ||||
| @ -44,12 +44,12 @@ func CheckConsistencyFor(t *testing.T, beansToCheck ...interface{}) { | ||||
| 
 | ||||
| 		for i := 0; i < sliceValue.Len(); i++ { | ||||
| 			entity := sliceValue.Index(i).Interface() | ||||
| 			checkable, ok := entity.(ConsistencyCheckable) | ||||
| 			checkable, ok := entity.(consistencyCheckable) | ||||
| 			if !ok { | ||||
| 				t.Errorf("Expected %+v (of type %T) to be checkable for consistency", | ||||
| 					entity, entity) | ||||
| 			} else { | ||||
| 				checkable.CheckForConsistency(t) | ||||
| 				checkable.checkForConsistency(t) | ||||
| 			} | ||||
| 		} | ||||
| 	} | ||||
| @ -68,7 +68,7 @@ func assertCount(t *testing.T, bean interface{}, expected int) { | ||||
| 		"Failed consistency test, the counted bean (of type %T) was %+v", bean, bean) | ||||
| } | ||||
| 
 | ||||
| func (user *User) CheckForConsistency(t *testing.T) { | ||||
| 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) | ||||
| @ -81,7 +81,7 @@ func (user *User) CheckForConsistency(t *testing.T) { | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func (repo *Repository) CheckForConsistency(t *testing.T) { | ||||
| 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) | ||||
| @ -112,7 +112,7 @@ func (repo *Repository) CheckForConsistency(t *testing.T) { | ||||
| 		"Unexpected number of closed milestones for repo %+v", repo) | ||||
| } | ||||
| 
 | ||||
| func (issue *Issue) CheckForConsistency(t *testing.T) { | ||||
| 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) | ||||
| @ -122,13 +122,13 @@ func (issue *Issue) CheckForConsistency(t *testing.T) { | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func (pr *PullRequest) CheckForConsistency(t *testing.T) { | ||||
| 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) { | ||||
| 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}) | ||||
| @ -136,7 +136,7 @@ func (milestone *Milestone) CheckForConsistency(t *testing.T) { | ||||
| 		"Unexpected number of closed issues for milestone %+v", milestone) | ||||
| } | ||||
| 
 | ||||
| func (label *Label) CheckForConsistency(t *testing.T) { | ||||
| 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), | ||||
| @ -155,12 +155,12 @@ func (label *Label) CheckForConsistency(t *testing.T) { | ||||
| 		"Unexpected number of closed issues for label %+v", label) | ||||
| } | ||||
| 
 | ||||
| func (team *Team) CheckForConsistency(t *testing.T) { | ||||
| 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) { | ||||
| 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) | ||||
| @ -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 | ||||
|  | ||||
| @ -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 | ||||
|  | ||||
							
								
								
									
										26
									
								
								models/main_test.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										26
									
								
								models/main_test.go
									
									
									
									
									
										Normal file
									
								
							| @ -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()) | ||||
| } | ||||
| @ -5,13 +5,8 @@ | ||||
| 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
 | ||||
| @ -19,24 +14,9 @@ import ( | ||||
| 	"gopkg.in/testfixtures.v2" | ||||
| ) | ||||
| 
 | ||||
| // NonexistentID an ID that will never exist
 | ||||
| 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 | ||||
| @ -52,6 +32,7 @@ func CreateTestEngine() error { | ||||
| 	return InitFixtures(&testfixtures.SQLite{}, "fixtures/") | ||||
| } | ||||
| 
 | ||||
| // TestFixturesAreConsistent assert that test fixtures are consistent
 | ||||
| func TestFixturesAreConsistent(t *testing.T) { | ||||
| 	assert.NoError(t, PrepareTestDatabase()) | ||||
| 	CheckConsistencyForAll(t) | ||||
| @ -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}, | ||||
|  | ||||
| @ -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) | ||||
| } | ||||
|  | ||||
							
								
								
									
										6
									
								
								vendor/github.com/go-xorm/builder/cond_in.go
									
									
									
										generated
									
									
										vendored
									
									
								
							
							
						
						
									
										6
									
								
								vendor/github.com/go-xorm/builder/cond_in.go
									
									
									
										generated
									
									
										vendored
									
									
								
							| @ -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 { | ||||
|  | ||||
							
								
								
									
										6
									
								
								vendor/github.com/go-xorm/builder/cond_notin.go
									
									
									
										generated
									
									
										vendored
									
									
								
							
							
						
						
									
										6
									
								
								vendor/github.com/go-xorm/builder/cond_notin.go
									
									
									
										generated
									
									
										vendored
									
									
								
							| @ -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 { | ||||
|  | ||||
							
								
								
									
										6
									
								
								vendor/vendor.json
									
									
									
									
										vendored
									
									
								
							
							
						
						
									
										6
									
								
								vendor/vendor.json
									
									
									
									
										vendored
									
									
								
							| @ -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=", | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user