Move transfer repository and rename repository on a service package a…

…nd start action notification (#8573)

* move transfer repository and rename repository on a service package and start action notification

* remove unused codes

* fix lint

* fix bugs

* fix test

* fix test

* fix test

* fix lint

* update go mod and sum

go.mod

@@ -80,7 +80,6 @@ require (
 	github.com/prometheus/client_golang v1.1.0
 	github.com/prometheus/procfs v0.0.4 // indirect
 	github.com/remyoudompheng/bigfft v0.0.0-20190321074620-2f0d2b0e0001 // indirect
-	github.com/russross/blackfriday v2.0.0+incompatible // indirect
 	github.com/russross/blackfriday/v2 v2.0.1
 	github.com/saintfish/chardet v0.0.0-20120816061221-3af4cd4741ca // indirect
 	github.com/satori/go.uuid v1.2.0

models/action.go

@@ -522,52 +522,6 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, bra
 	return nil
 }
 
-func transferRepoAction(e Engine, doer, oldOwner *User, repo *Repository) (err error) {
-	if err = notifyWatchers(e, &Action{
-		ActUserID: doer.ID,
-		ActUser:   doer,
-		OpType:    ActionTransferRepo,
-		RepoID:    repo.ID,
-		Repo:      repo,
-		IsPrivate: repo.IsPrivate,
-		Content:   path.Join(oldOwner.Name, repo.Name),
-	}); err != nil {
-		return fmt.Errorf("notifyWatchers: %v", err)
-	}
-
-	// Remove watch for organization.
-	if oldOwner.IsOrganization() {
-		if err = watchRepo(e, oldOwner.ID, repo.ID, false); err != nil {
-			return fmt.Errorf("watchRepo [false]: %v", err)
-		}
-	}
-
-	return nil
-}
-
-// TransferRepoAction adds new action for transferring repository,
-// the Owner field of repository is assumed to be new owner.
-func TransferRepoAction(doer, oldOwner *User, repo *Repository) error {
-	return transferRepoAction(x, doer, oldOwner, repo)
-}
-
-func mergePullRequestAction(e Engine, doer *User, repo *Repository, issue *Issue) error {
-	return notifyWatchers(e, &Action{
-		ActUserID: doer.ID,
-		ActUser:   doer,
-		OpType:    ActionMergePullRequest,
-		Content:   fmt.Sprintf("%d|%s", issue.Index, issue.Title),
-		RepoID:    repo.ID,
-		Repo:      repo,
-		IsPrivate: repo.IsPrivate,
-	})
-}
-
-// MergePullRequestAction adds new action for merging pull request.
-func MergePullRequestAction(actUser *User, repo *Repository, pull *Issue) error {
-	return mergePullRequestAction(x, actUser, repo, pull)
-}
-
 // GetFeedsOptions options for retrieving feeds
 type GetFeedsOptions struct {
 	RequestedUser    *User

models/action_test.go

@@ -332,54 +332,6 @@ func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) {
 	CheckConsistencyFor(t, &Action{})
 }
 
-func TestTransferRepoAction(t *testing.T) {
-	assert.NoError(t, PrepareTestDatabase())
-
-	user2 := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
-	user4 := AssertExistsAndLoadBean(t, &User{ID: 4}).(*User)
-	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1, OwnerID: user2.ID}).(*Repository)
-
-	repo.OwnerID = user4.ID
-	repo.Owner = user4
-
-	actionBean := &Action{
-		OpType:    ActionTransferRepo,
-		ActUserID: user2.ID,
-		ActUser:   user2,
-		RepoID:    repo.ID,
-		Repo:      repo,
-		IsPrivate: repo.IsPrivate,
-	}
-	AssertNotExistsBean(t, actionBean)
-	assert.NoError(t, TransferRepoAction(user2, user2, repo))
-	AssertExistsAndLoadBean(t, actionBean)
-
-	_, err := x.ID(repo.ID).Cols("owner_id").Update(repo)
-	assert.NoError(t, err)
-	CheckConsistencyFor(t, &Action{})
-}
-
-func TestMergePullRequestAction(t *testing.T) {
-	assert.NoError(t, PrepareTestDatabase())
-	user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
-	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1, OwnerID: user.ID}).(*Repository)
-	repo.Owner = user
-	issue := AssertExistsAndLoadBean(t, &Issue{ID: 3, RepoID: repo.ID}).(*Issue)
-
-	actionBean := &Action{
-		OpType:    ActionMergePullRequest,
-		ActUserID: user.ID,
-		ActUser:   user,
-		RepoID:    repo.ID,
-		Repo:      repo,
-		IsPrivate: repo.IsPrivate,
-	}
-	AssertNotExistsBean(t, actionBean)
-	assert.NoError(t, MergePullRequestAction(user, repo, issue))
-	AssertExistsAndLoadBean(t, actionBean)
-	CheckConsistencyFor(t, &Action{})
-}
-
 func TestGetFeeds(t *testing.T) {
 	// test with an individual user
 	assert.NoError(t, PrepareTestDatabase())

models/repo.go

@@ -1792,8 +1792,13 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error
 
 	if err = watchRepo(sess, doer.ID, repo.ID, true); err != nil {
 		return fmt.Errorf("watchRepo: %v", err)
-	} else if err = transferRepoAction(sess, doer, owner, repo); err != nil {
-		return fmt.Errorf("transferRepoAction: %v", err)
+	}
+
+	// Remove watch for organization.
+	if owner.IsOrganization() {
+		if err = watchRepo(sess, owner.ID, repo.ID, false); err != nil {
+			return fmt.Errorf("watchRepo [false]: %v", err)
+		}
 	}
 
 	// Rename remote repository to new path and delete local copy.
@@ -1824,23 +1829,21 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) error
 }
 
 // ChangeRepositoryName changes all corresponding setting from old repository name to new one.
-func ChangeRepositoryName(u *User, oldRepoName, newRepoName string) (err error) {
-	oldRepoName = strings.ToLower(oldRepoName)
+func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err error) {
 	newRepoName = strings.ToLower(newRepoName)
 	if err = IsUsableRepoName(newRepoName); err != nil {
 		return err
 	}
 
-	has, err := IsRepositoryExist(u, newRepoName)
-	if err != nil {
-		return fmt.Errorf("IsRepositoryExist: %v", err)
-	} else if has {
-		return ErrRepoAlreadyExist{u.Name, newRepoName}
+	if err := repo.GetOwner(); err != nil {
+		return err
 	}
 
-	repo, err := GetRepositoryByName(u.ID, oldRepoName)
+	has, err := IsRepositoryExist(repo.Owner, newRepoName)
 	if err != nil {
-		return fmt.Errorf("GetRepositoryByName: %v", err)
+		return fmt.Errorf("IsRepositoryExist: %v", err)
+	} else if has {
+		return ErrRepoAlreadyExist{repo.Owner.Name, newRepoName}
 	}
 
 	// Change repository directory name. We must lock the local copy of the
@@ -1849,14 +1852,14 @@ func ChangeRepositoryName(u *User, oldRepoName, newRepoName string) (err error)
 	repoWorkingPool.CheckIn(com.ToStr(repo.ID))
 	defer repoWorkingPool.CheckOut(com.ToStr(repo.ID))
 
-	newRepoPath := RepoPath(u.Name, newRepoName)
+	newRepoPath := RepoPath(repo.Owner.Name, newRepoName)
 	if err = os.Rename(repo.RepoPath(), newRepoPath); err != nil {
 		return fmt.Errorf("rename repository directory: %v", err)
 	}
 
 	wikiPath := repo.WikiPath()
 	if com.IsExist(wikiPath) {
-		if err = os.Rename(wikiPath, WikiPath(u.Name, newRepoName)); err != nil {
+		if err = os.Rename(wikiPath, WikiPath(repo.Owner.Name, newRepoName)); err != nil {
 			return fmt.Errorf("rename repository wiki: %v", err)
 		}
 	}
@@ -1868,7 +1871,7 @@ func ChangeRepositoryName(u *User, oldRepoName, newRepoName string) (err error)
 	}
 
 	// If there was previously a redirect at this location, remove it.
-	if err = deleteRepoRedirect(sess, u.ID, newRepoName); err != nil {
+	if err = deleteRepoRedirect(sess, repo.OwnerID, newRepoName); err != nil {
 		return fmt.Errorf("delete repo redirect: %v", err)
 	}
 

models/repo_test.go

@@ -15,7 +15,6 @@ import (
 	"code.gitea.io/gitea/modules/markup"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/unknwon/com"
 )
 
 func TestRepo(t *testing.T) {
@@ -142,29 +141,6 @@ func TestRepoAPIURL(t *testing.T) {
 	assert.Equal(t, "https://try.gitea.io/api/v1/repos/user12/repo10", repo.APIURL())
 }
 
-func TestTransferOwnership(t *testing.T) {
-	assert.NoError(t, PrepareTestDatabase())
-
-	doer := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
-	repo := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
-	repo.Owner = AssertExistsAndLoadBean(t, &User{ID: repo.OwnerID}).(*User)
-	assert.NoError(t, TransferOwnership(doer, "user2", repo))
-
-	transferredRepo := AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
-	assert.EqualValues(t, 2, transferredRepo.OwnerID)
-
-	assert.False(t, com.IsExist(RepoPath("user3", "repo3")))
-	assert.True(t, com.IsExist(RepoPath("user2", "repo3")))
-	AssertExistsAndLoadBean(t, &Action{
-		OpType:    ActionTransferRepo,
-		ActUserID: 2,
-		RepoID:    3,
-		Content:   "user3/repo3",
-	})
-
-	CheckConsistencyFor(t, &Repository{}, &User{}, &Team{})
-}
-
 func TestUploadAvatar(t *testing.T) {
 
 	// Generate image

modules/notification/action/action.go

@@ -6,6 +6,7 @@ package action
 
 import (
 	"fmt"
+	"path"
 	"strings"
 
 	"code.gitea.io/gitea/models"
@@ -77,19 +78,33 @@ func (a *actionNotifier) NotifyNewPullRequest(pull *models.PullRequest) {
 	}
 }
 
-func (a *actionNotifier) NotifyRenameRepository(doer *models.User, repo *models.Repository, oldName string) {
+func (a *actionNotifier) NotifyRenameRepository(doer *models.User, repo *models.Repository, oldRepoName string) {
+	log.Trace("action.ChangeRepositoryName: %s/%s", doer.Name, repo.Name)
+
 	if err := models.NotifyWatchers(&models.Action{
 		ActUserID: doer.ID,
 		ActUser:   doer,
 		OpType:    models.ActionRenameRepo,
 		RepoID:    repo.ID,
 		Repo:      repo,
 		IsPrivate: repo.IsPrivate,
-		Content:   oldName,
+		Content:   oldRepoName,
 	}); err != nil {
-		log.Error("notify watchers: %v", err)
-	} else {
-		log.Trace("action.renameRepoAction: %s/%s", doer.Name, repo.Name)
+		log.Error("NotifyWatchers: %v", err)
+	}
+}
+
+func (a *actionNotifier) NotifyTransferRepository(doer *models.User, repo *models.Repository, oldOwnerName string) {
+	if err := models.NotifyWatchers(&models.Action{
+		ActUserID: doer.ID,
+		ActUser:   doer,
+		OpType:    models.ActionTransferRepo,
+		RepoID:    repo.ID,
+		Repo:      repo,
+		IsPrivate: repo.IsPrivate,
+		Content:   path.Join(oldOwnerName, repo.Name),
+	}); err != nil {
+		log.Error("NotifyWatchers: %v", err)
 	}
 }
 

modules/notification/base/notifier.go

@@ -17,7 +17,8 @@ type Notifier interface {
 	NotifyMigrateRepository(doer *models.User, u *models.User, repo *models.Repository)
 	NotifyDeleteRepository(doer *models.User, repo *models.Repository)
 	NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository)
-	NotifyRenameRepository(doer *models.User, repo *models.Repository, oldName string)
+	NotifyRenameRepository(doer *models.User, repo *models.Repository, oldRepoName string)
+	NotifyTransferRepository(doer *models.User, repo *models.Repository, oldOwnerName string)
 
 	NotifyNewIssue(*models.Issue)
 	NotifyIssueChangeStatus(*models.User, *models.Issue, bool)

modules/notification/base/null.go

@@ -58,18 +58,6 @@ func (*NullNotifier) NotifyUpdateComment(doer *models.User, c *models.Comment, o
 func (*NullNotifier) NotifyDeleteComment(doer *models.User, c *models.Comment) {
 }
 
-// NotifyDeleteRepository places a place holder function
-func (*NullNotifier) NotifyDeleteRepository(doer *models.User, repo *models.Repository) {
-}
-
-// NotifyForkRepository places a place holder function
-func (*NullNotifier) NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) {
-}
-
-// NotifyRenameRepository places a place holder function
-func (*NullNotifier) NotifyRenameRepository(doer *models.User, repo *models.Repository, oldName string) {
-}
-
 // NotifyNewRelease places a place holder function
 func (*NullNotifier) NotifyNewRelease(rel *models.Release) {
 }
@@ -111,6 +99,14 @@ func (*NullNotifier) NotifyIssueChangeLabels(doer *models.User, issue *models.Is
 func (*NullNotifier) NotifyCreateRepository(doer *models.User, u *models.User, repo *models.Repository) {
 }
 
+// NotifyDeleteRepository places a place holder function
+func (*NullNotifier) NotifyDeleteRepository(doer *models.User, repo *models.Repository) {
+}
+
+// NotifyForkRepository places a place holder function
+func (*NullNotifier) NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) {
+}
+
 // NotifyMigrateRepository places a place holder function
 func (*NullNotifier) NotifyMigrateRepository(doer *models.User, u *models.User, repo *models.Repository) {
 }
@@ -126,3 +122,11 @@ func (*NullNotifier) NotifyCreateRef(doer *models.User, repo *models.Repository,
 // NotifyDeleteRef notifies branch or tag deleteion to notifiers
 func (*NullNotifier) NotifyDeleteRef(doer *models.User, repo *models.Repository, refType, refFullName string) {
 }
+
+// NotifyRenameRepository places a place holder function
+func (*NullNotifier) NotifyRenameRepository(doer *models.User, repo *models.Repository, oldRepoName string) {
+}
+
+// NotifyTransferRepository places a place holder function
+func (*NullNotifier) NotifyTransferRepository(doer *models.User, repo *models.Repository, oldOwnerName string) {
+}

modules/notification/notification.go

@@ -101,27 +101,6 @@ func NotifyDeleteComment(doer *models.User, c *models.Comment) {
 	}
 }
 
-// NotifyDeleteRepository notifies delete repository to notifiers
-func NotifyDeleteRepository(doer *models.User, repo *models.Repository) {
-	for _, notifier := range notifiers {
-		notifier.NotifyDeleteRepository(doer, repo)
-	}
-}
-
-// NotifyForkRepository notifies fork repository to notifiers
-func NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) {
-	for _, notifier := range notifiers {
-		notifier.NotifyForkRepository(doer, oldRepo, repo)
-	}
-}
-
-// NotifyRenameRepository notifies repository renamed
-func NotifyRenameRepository(doer *models.User, repo *models.Repository, oldName string) {
-	for _, notifier := range notifiers {
-		notifier.NotifyRenameRepository(doer, repo, oldName)
-	}
-}
-
 // NotifyNewRelease notifies new release to notifiers
 func NotifyNewRelease(rel *models.Release) {
 	for _, notifier := range notifiers {
@@ -200,6 +179,34 @@ func NotifyMigrateRepository(doer *models.User, u *models.User, repo *models.Rep
 	}
 }
 
+// NotifyTransferRepository notifies create repository to notifiers
+func NotifyTransferRepository(doer *models.User, repo *models.Repository, newOwnerName string) {
+	for _, notifier := range notifiers {
+		notifier.NotifyTransferRepository(doer, repo, newOwnerName)
+	}
+}
+
+// NotifyDeleteRepository notifies delete repository to notifiers
+func NotifyDeleteRepository(doer *models.User, repo *models.Repository) {
+	for _, notifier := range notifiers {
+		notifier.NotifyDeleteRepository(doer, repo)
+	}
+}
+
+// NotifyForkRepository notifies fork repository to notifiers
+func NotifyForkRepository(doer *models.User, oldRepo, repo *models.Repository) {
+	for _, notifier := range notifiers {
+		notifier.NotifyForkRepository(doer, oldRepo, repo)
+	}
+}
+
+// NotifyRenameRepository notifies repository renamed
+func NotifyRenameRepository(doer *models.User, repo *models.Repository, oldName string) {
+	for _, notifier := range notifiers {
+		notifier.NotifyRenameRepository(doer, repo, oldName)
+	}
+}
+
 // NotifyPushCommits notifies commits pushed to notifiers
 func NotifyPushCommits(pusher *models.User, repo *models.Repository, refName, oldCommitID, newCommitID string, commits *models.PushCommits) {
 	for _, notifier := range notifiers {