diff --git a/models/pull.go b/models/pull.go index 27824c546..388ee16b4 100644 --- a/models/pull.go +++ b/models/pull.go @@ -609,8 +609,8 @@ func (pr *PullRequest) testPatch(e Engine) (err error) { return nil } - repoWorkingPool.CheckIn(com.ToStr(pr.BaseRepoID)) - defer repoWorkingPool.CheckOut(com.ToStr(pr.BaseRepoID)) + RepoWorkingPool.CheckIn(com.ToStr(pr.BaseRepoID)) + defer RepoWorkingPool.CheckOut(com.ToStr(pr.BaseRepoID)) log.Trace("PullRequest[%d].testPatch (patchPath): %s", pr.ID, patchPath) diff --git a/models/repo.go b/models/repo.go index 347c90957..6a260d393 100644 --- a/models/repo.go +++ b/models/repo.go @@ -43,7 +43,8 @@ import ( "xorm.io/builder" ) -var repoWorkingPool = sync.NewExclusivePool() +// RepoWorkingPool represents a working pool to order the parallel changes to the same repository +var RepoWorkingPool = sync.NewExclusivePool() var ( // ErrMirrorNotExist mirror does not exist error @@ -1655,7 +1656,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error return fmt.Errorf("sess.Begin: %v", err) } - owner := repo.Owner + oldOwner := repo.Owner // Note: we have to set value here to make sure recalculate accesses is based on // new owner. @@ -1691,8 +1692,8 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error } // Remove old team-repository relations. - if owner.IsOrganization() { - if err = owner.removeOrgRepo(sess, repo.ID); err != nil { + if oldOwner.IsOrganization() { + if err = oldOwner.removeOrgRepo(sess, repo.ID); err != nil { return fmt.Errorf("removeOrgRepo: %v", err) } } @@ -1716,7 +1717,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error // Update repository count. if _, err = sess.Exec("UPDATE `user` SET num_repos=num_repos+1 WHERE id=?", newOwner.ID); err != nil { return fmt.Errorf("increase new owner repository count: %v", err) - } else if _, err = sess.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", owner.ID); err != nil { + } else if _, err = sess.Exec("UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", oldOwner.ID); err != nil { return fmt.Errorf("decrease old owner repository count: %v", err) } @@ -1725,8 +1726,8 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error } // Remove watch for organization. - if owner.IsOrganization() { - if err = watchRepo(sess, owner.ID, repo.ID, false); err != nil { + if oldOwner.IsOrganization() { + if err = watchRepo(sess, oldOwner.ID, repo.ID, false); err != nil { return fmt.Errorf("watchRepo [false]: %v", err) } } @@ -1738,12 +1739,12 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error return fmt.Errorf("Failed to create dir %s: %v", dir, err) } - if err = os.Rename(RepoPath(owner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil { + if err = os.Rename(RepoPath(oldOwner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil { return fmt.Errorf("rename repository directory: %v", err) } // Rename remote wiki repository to new path and delete local copy. - wikiPath := WikiPath(owner.Name, repo.Name) + wikiPath := WikiPath(oldOwner.Name, repo.Name) if com.IsExist(wikiPath) { if err = os.Rename(wikiPath, WikiPath(newOwner.Name, repo.Name)); err != nil { return fmt.Errorf("rename repository wiki: %v", err) @@ -1755,11 +1756,16 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error return fmt.Errorf("delete repo redirect: %v", err) } + if err := NewRepoRedirect(DBContext{sess}, oldOwner.ID, repo.ID, repo.Name, repo.Name); err != nil { + return fmt.Errorf("NewRepoRedirect: %v", err) + } + return sess.Commit() } // ChangeRepositoryName changes all corresponding setting from old repository name to new one. func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err error) { + oldRepoName := repo.Name newRepoName = strings.ToLower(newRepoName) if err = IsUsableRepoName(newRepoName); err != nil { return err @@ -1776,12 +1782,6 @@ func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err return ErrRepoAlreadyExist{repo.Owner.Name, newRepoName} } - // Change repository directory name. We must lock the local copy of the - // repo so that we can atomically rename the repo path and updates the - // local copy's origin accordingly. - repoWorkingPool.CheckIn(com.ToStr(repo.ID)) - defer repoWorkingPool.CheckOut(com.ToStr(repo.ID)) - newRepoPath := RepoPath(repo.Owner.Name, newRepoName) if err = os.Rename(repo.RepoPath(), newRepoPath); err != nil { return fmt.Errorf("rename repository directory: %v", err) @@ -1805,6 +1805,10 @@ func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err return fmt.Errorf("delete repo redirect: %v", err) } + if err := NewRepoRedirect(DBContext{sess}, repo.Owner.ID, repo.ID, oldRepoName, newRepoName); err != nil { + return err + } + return sess.Commit() } diff --git a/models/repo_redirect.go b/models/repo_redirect.go index 2714121a6..182b6b41a 100644 --- a/models/repo_redirect.go +++ b/models/repo_redirect.go @@ -6,8 +6,6 @@ package models import ( "strings" - - "code.gitea.io/gitea/modules/log" ) // RepoRedirect represents that a repo name should be redirected to another @@ -31,36 +29,22 @@ func LookupRepoRedirect(ownerID int64, repoName string) (int64, error) { } // NewRepoRedirect create a new repo redirect -func NewRepoRedirect(ownerID, repoID int64, oldRepoName, newRepoName string) error { +func NewRepoRedirect(ctx DBContext, ownerID, repoID int64, oldRepoName, newRepoName string) error { oldRepoName = strings.ToLower(oldRepoName) newRepoName = strings.ToLower(newRepoName) - sess := x.NewSession() - defer sess.Close() - if err := sess.Begin(); err != nil { + if err := deleteRepoRedirect(ctx.e, ownerID, newRepoName); err != nil { return err } - if err := deleteRepoRedirect(sess, ownerID, newRepoName); err != nil { - errRollback := sess.Rollback() - if errRollback != nil { - log.Error("NewRepoRedirect sess.Rollback: %v", errRollback) - } - return err - } - - if _, err := sess.Insert(&RepoRedirect{ + if _, err := ctx.e.Insert(&RepoRedirect{ OwnerID: ownerID, LowerName: oldRepoName, RedirectRepoID: repoID, }); err != nil { - errRollback := sess.Rollback() - if errRollback != nil { - log.Error("NewRepoRedirect sess.Rollback: %v", errRollback) - } return err } - return sess.Commit() + return nil } // deleteRepoRedirect delete any redirect from the specified repo name to diff --git a/models/repo_redirect_test.go b/models/repo_redirect_test.go index b3da32836..44ec2b4e9 100644 --- a/models/repo_redirect_test.go +++ b/models/repo_redirect_test.go @@ -26,7 +26,7 @@ func TestNewRepoRedirect(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) - assert.NoError(t, NewRepoRedirect(repo.OwnerID, repo.ID, repo.Name, "newreponame")) + assert.NoError(t, NewRepoRedirect(DefaultDBContext(), repo.OwnerID, repo.ID, repo.Name, "newreponame")) AssertExistsAndLoadBean(t, &RepoRedirect{ OwnerID: repo.OwnerID, @@ -45,7 +45,7 @@ func TestNewRepoRedirect2(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) - assert.NoError(t, NewRepoRedirect(repo.OwnerID, repo.ID, repo.Name, "oldrepo1")) + assert.NoError(t, NewRepoRedirect(DefaultDBContext(), repo.OwnerID, repo.ID, repo.Name, "oldrepo1")) AssertExistsAndLoadBean(t, &RepoRedirect{ OwnerID: repo.OwnerID, @@ -64,7 +64,7 @@ func TestNewRepoRedirect3(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) repo := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository) - assert.NoError(t, NewRepoRedirect(repo.OwnerID, repo.ID, repo.Name, "newreponame")) + assert.NoError(t, NewRepoRedirect(DefaultDBContext(), repo.OwnerID, repo.ID, repo.Name, "newreponame")) AssertExistsAndLoadBean(t, &RepoRedirect{ OwnerID: repo.OwnerID, diff --git a/services/repository/transfer.go b/services/repository/transfer.go index 2d20dcd1a..9b3c084ea 100644 --- a/services/repository/transfer.go +++ b/services/repository/transfer.go @@ -5,10 +5,10 @@ package repository import ( - "fmt" - "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/notification" + + "github.com/unknwon/com" ) // TransferOwnership transfers all corresponding setting from old user to new one. @@ -19,13 +19,12 @@ func TransferOwnership(doer *models.User, newOwnerName string, repo *models.Repo oldOwner := repo.Owner + models.RepoWorkingPool.CheckIn(com.ToStr(repo.ID)) if err := models.TransferOwnership(doer, newOwnerName, repo); err != nil { + models.RepoWorkingPool.CheckOut(com.ToStr(repo.ID)) return err } - - if err := models.NewRepoRedirect(oldOwner.ID, repo.ID, repo.Name, repo.Name); err != nil { - return fmt.Errorf("NewRepoRedirect: %v", err) - } + models.RepoWorkingPool.CheckOut(com.ToStr(repo.ID)) notification.NotifyTransferRepository(doer, repo, oldOwner.Name) @@ -36,17 +35,16 @@ func TransferOwnership(doer *models.User, newOwnerName string, repo *models.Repo func ChangeRepositoryName(doer *models.User, repo *models.Repository, newRepoName string) error { oldRepoName := repo.Name + // Change repository directory name. We must lock the local copy of the + // repo so that we can atomically rename the repo path and updates the + // local copy's origin accordingly. + + models.RepoWorkingPool.CheckIn(com.ToStr(repo.ID)) if err := models.ChangeRepositoryName(doer, repo, newRepoName); err != nil { + models.RepoWorkingPool.CheckOut(com.ToStr(repo.ID)) return err } - - if err := repo.GetOwner(); err != nil { - return err - } - - if err := models.NewRepoRedirect(repo.Owner.ID, repo.ID, oldRepoName, newRepoName); err != nil { - return err - } + models.RepoWorkingPool.CheckOut(com.ToStr(repo.ID)) notification.NotifyRenameRepository(doer, repo, oldRepoName)