Skip to content

Commit 83c5adc

Browse files
lunnylunny
committed
refactor transfer related code
1 parent 5c150ce commit 83c5adc

File tree

8 files changed

+123
-156
lines changed

8 files changed

+123
-156
lines changed

‎models/repo/transfer.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,10 @@ func (r *RepoTransfer) LoadAttributes(ctx context.Context) error {
117117
return nil
118118
}
119119

120-
// CanUserAcceptTransfer checks if the user has the rights to accept/decline a repo transfer.
120+
// CanUserAcceptOrRejectTransfer checks if the user has the rights to accept/decline a repo transfer.
121121
// For user, it checks if it's himself
122122
// For organizations, it checks if the user is able to create repos
123-
func (r *RepoTransfer) CanUserAcceptTransfer(ctx context.Context, u *user_model.User) bool {
123+
func (r *RepoTransfer) CanUserAcceptOrRejectTransfer(ctx context.Context, u *user_model.User) bool {
124124
if err := r.LoadAttributes(ctx); err != nil {
125125
log.Error("LoadAttributes: %v", err)
126126
return false

‎routers/api/v1/repo/transfer.go

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ func AcceptTransfer(ctx *context.APIContext) {
161161
// "404":
162162
// "$ref": "#/responses/notFound"
163163

164-
err := acceptOrRejectRepoTransfer(ctx, true)
164+
err := repo_service.AcceptTransferOwnership(ctx, ctx.Repo.Repository, ctx.Doer)
165165
if ctx.Written() {
166166
return
167167
}
@@ -199,40 +199,11 @@ func RejectTransfer(ctx *context.APIContext) {
199199
// "404":
200200
// "$ref": "#/responses/notFound"
201201

202-
err := acceptOrRejectRepoTransfer(ctx, false)
203-
if ctx.Written() {
204-
return
205-
}
202+
err := repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
206203
if err != nil {
207204
ctx.Error(http.StatusInternalServerError, "acceptOrRejectRepoTransfer", err)
208205
return
209206
}
210207

211208
ctx.JSON(http.StatusOK, convert.ToRepo(ctx, ctx.Repo.Repository, ctx.Repo.Permission))
212209
}
213-
214-
func acceptOrRejectRepoTransfer(ctx *context.APIContext, accept bool) error {
215-
repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, ctx.Repo.Repository)
216-
if err != nil {
217-
if repo_model.IsErrNoPendingTransfer(err) {
218-
ctx.NotFound()
219-
return nil
220-
}
221-
return err
222-
}
223-
224-
if err := repoTransfer.LoadAttributes(ctx); err != nil {
225-
return err
226-
}
227-
228-
if !repoTransfer.CanUserAcceptTransfer(ctx, ctx.Doer) {
229-
ctx.Error(http.StatusForbidden, "CanUserAcceptTransfer", nil)
230-
return fmt.Errorf("user does not have permissions to do this")
231-
}
232-
233-
if accept {
234-
return repo_service.TransferOwnership(ctx, repoTransfer.Doer, repoTransfer.Recipient, ctx.Repo.Repository, repoTransfer.Teams)
235-
}
236-
237-
return repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository)
238-
}

‎routers/web/repo/repo.go

Lines changed: 13 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -322,9 +322,17 @@ func Action(ctx *context.Context) {
322322
case "unstar":
323323
err = repo_model.StarRepo(ctx, ctx.Doer, ctx.Repo.Repository, false)
324324
case "accept_transfer":
325-
err = acceptOrRejectRepoTransfer(ctx, true)
325+
err = repo_service.AcceptTransferOwnership(ctx, ctx.Repo.Repository, ctx.Doer)
326+
if err == nil {
327+
ctx.Flash.Success(ctx.Tr("repo.settings.transfer.success"))
328+
}
329+
ctx.Redirect(ctx.Repo.Repository.Link())
326330
case "reject_transfer":
327-
err = acceptOrRejectRepoTransfer(ctx, false)
331+
err = repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
332+
if err == nil {
333+
ctx.Flash.Success(ctx.Tr("repo.settings.transfer.rejected"))
334+
}
335+
ctx.Redirect(ctx.Repo.Repository.Link())
328336
case "desc": // FIXME: this is not used
329337
if !ctx.Repo.IsOwner() {
330338
ctx.Error(http.StatusNotFound)
@@ -339,6 +347,9 @@ func Action(ctx *context.Context) {
339347
if err != nil {
340348
if errors.Is(err, user_model.ErrBlockedUser) {
341349
ctx.Flash.Error(ctx.Tr("repo.action.blocked_user"))
350+
} else if errors.Is(err, util.ErrPermissionDenied) {
351+
ctx.Error(http.StatusNotFound)
352+
return
342353
} else {
343354
ctx.ServerError(fmt.Sprintf("Action (%s)", ctx.PathParam("action")), err)
344355
return
@@ -377,41 +388,6 @@ func Action(ctx *context.Context) {
377388
ctx.RedirectToCurrentSite(ctx.FormString("redirect_to"), ctx.Repo.RepoLink)
378389
}
379390

380-
func acceptOrRejectRepoTransfer(ctx *context.Context, accept bool) error {
381-
repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, ctx.Repo.Repository)
382-
if err != nil {
383-
return err
384-
}
385-
386-
if err := repoTransfer.LoadAttributes(ctx); err != nil {
387-
return err
388-
}
389-
390-
if !repoTransfer.CanUserAcceptTransfer(ctx, ctx.Doer) {
391-
return errors.New("user does not have enough permissions")
392-
}
393-
394-
if accept {
395-
if ctx.Repo.GitRepo != nil {
396-
ctx.Repo.GitRepo.Close()
397-
ctx.Repo.GitRepo = nil
398-
}
399-
400-
if err := repo_service.TransferOwnership(ctx, repoTransfer.Doer, repoTransfer.Recipient, ctx.Repo.Repository, repoTransfer.Teams); err != nil {
401-
return err
402-
}
403-
ctx.Flash.Success(ctx.Tr("repo.settings.transfer.success"))
404-
} else {
405-
if err := repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository); err != nil {
406-
return err
407-
}
408-
ctx.Flash.Success(ctx.Tr("repo.settings.transfer.rejected"))
409-
}
410-
411-
ctx.Redirect(ctx.Repo.Repository.Link())
412-
return nil
413-
}
414-
415391
// RedirectDownload return a file based on the following infos:
416392
func RedirectDownload(ctx *context.Context) {
417393
var (

‎routers/web/repo/setting/setting.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,7 @@ func SettingsPost(ctx *context.Context) {
830830
return
831831
}
832832

833-
if err := repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository); err != nil {
833+
if err := repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer); err != nil {
834834
ctx.ServerError("CancelRepositoryTransfer", err)
835835
return
836836
}

‎services/context/repo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,7 @@ func RepoAssignment(ctx *Context) {
719719

720720
ctx.Data["RepoTransfer"] = repoTransfer
721721
if ctx.Doer != nil {
722-
ctx.Data["CanUserAcceptTransfer"] = repoTransfer.CanUserAcceptTransfer(ctx, ctx.Doer)
722+
ctx.Data["CanUserAcceptTransfer"] = repoTransfer.CanUserAcceptOrRejectTransfer(ctx, ctx.Doer)
723723
}
724724
}
725725

‎services/repository/transfer.go

Lines changed: 97 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -27,49 +27,51 @@ func getRepoWorkingLockKey(repoID int64) string {
2727
return fmt.Sprintf("repo_working_%d", repoID)
2828
}
2929

30-
// TransferOwnership transfers all corresponding setting from old user to new one.
31-
func TransferOwnership(ctx context.Context, doer, newOwner *user_model.User, repo *repo_model.Repository, teams []*organization.Team) error {
32-
if err := repo.LoadOwner(ctx); err != nil {
33-
return err
34-
}
35-
for _, team := range teams {
36-
if newOwner.ID != team.OrgID {
37-
return fmt.Errorf("team %d does not belong to organization", team.ID)
38-
}
39-
}
40-
41-
oldOwner := repo.Owner
42-
30+
// AcceptTransferOwnership transfers all corresponding setting from old user to new one.
31+
func AcceptTransferOwnership(ctx context.Context, repo *repo_model.Repository, doer *user_model.User) error {
4332
releaser, err := globallock.Lock(ctx, getRepoWorkingLockKey(repo.ID))
4433
if err != nil {
4534
log.Error("lock.Lock(): %v", err)
4635
return fmt.Errorf("lock.Lock: %w", err)
4736
}
4837
defer releaser()
4938

50-
if err := transferOwnership(ctx, doer, newOwner.Name, repo); err != nil {
51-
return err
52-
}
53-
releaser()
54-
55-
newRepo, err := repo_model.GetRepositoryByID(ctx, repo.ID)
39+
repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, repo)
5640
if err != nil {
5741
return err
5842
}
5943

60-
for _, team := range teams {
61-
if err := addRepositoryToTeam(ctx, team, newRepo); err != nil {
44+
if err := db.WithTx(ctx, func(ctx context.Context) error {
45+
if err := repoTransfer.LoadAttributes(ctx); err != nil {
6246
return err
6347
}
48+
49+
if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) {
50+
return util.ErrPermissionDenied
51+
}
52+
53+
if err := repo.LoadOwner(ctx); err != nil {
54+
return err
55+
}
56+
for _, team := range repoTransfer.Teams {
57+
if repoTransfer.Recipient.ID != team.OrgID {
58+
return fmt.Errorf("team %d does not belong to organization", team.ID)
59+
}
60+
}
61+
62+
return transferOwnership(ctx, repoTransfer.Doer, repoTransfer.Recipient.Name, repo, repoTransfer.Teams)
63+
}); err != nil {
64+
return err
6465
}
66+
releaser()
6567

66-
notify_service.TransferRepository(ctx, doer, repo, oldOwner.Name)
68+
notify_service.TransferRepository(ctx, doer, repo, repoTransfer.Recipient.Name)
6769

6870
return nil
6971
}
7072

7173
// transferOwnership transfers all corresponding repository items from old user to new one.
72-
func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName string, repo *repo_model.Repository) (err error) {
74+
func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName string, repo *repo_model.Repository, teams []*organization.Team) (err error) {
7375
repoRenamed := false
7476
wikiRenamed := false
7577
oldOwnerName := doer.Name
@@ -301,6 +303,17 @@ func transferOwnership(ctx context.Context, doer *user_model.User, newOwnerName
301303
return fmt.Errorf("repo_model.NewRedirect: %w", err)
302304
}
303305

306+
newRepo, err := repo_model.GetRepositoryByID(ctx, repo.ID)
307+
if err != nil {
308+
return err
309+
}
310+
311+
for _, team := range teams {
312+
if err := addRepositoryToTeam(ctx, team, newRepo); err != nil {
313+
return err
314+
}
315+
}
316+
304317
return committer.Commit()
305318
}
306319

@@ -343,17 +356,9 @@ func changeRepositoryName(ctx context.Context, repo *repo_model.Repository, newR
343356
}
344357
}
345358

346-
ctx, committer, err := db.TxContext(ctx)
347-
if err != nil {
348-
return err
349-
}
350-
defer committer.Close()
351-
352-
if err := repo_model.NewRedirect(ctx, repo.Owner.ID, repo.ID, oldRepoName, newRepoName); err != nil {
353-
return err
354-
}
355-
356-
return committer.Commit()
359+
return db.WithTx(ctx, func(ctx context.Context) error {
360+
return repo_model.NewRedirect(ctx, repo.Owner.ID, repo.ID, oldRepoName, newRepoName)
361+
})
357362
}
358363

359364
// ChangeRepositoryName changes all corresponding setting from old repository name to new one.
@@ -391,66 +396,81 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use
391396
return err
392397
}
393398

394-
// Admin is always allowed to transfer || user transfer repo back to his account
395-
if doer.IsAdmin || doer.ID == newOwner.ID {
396-
return TransferOwnership(ctx, doer, newOwner, repo, teams)
397-
}
399+
var isTransfer bool
398400

399-
if user_model.IsUserBlockedBy(ctx, doer, newOwner.ID) {
400-
return user_model.ErrBlockedUser
401-
}
401+
if err := db.WithTx(ctx, func(ctx context.Context) error {
402+
// Admin is always allowed to transfer || user transfer repo back to his account
403+
if doer.IsAdmin || doer.ID == newOwner.ID {
404+
isTransfer = true
405+
return transferOwnership(ctx, doer, newOwner.Name, repo, teams)
406+
}
402407

403-
// If new owner is an org and user can create repos he can transfer directly too
404-
if newOwner.IsOrganization() {
405-
allowed, err := organization.CanCreateOrgRepo(ctx, newOwner.ID, doer.ID)
406-
if err != nil {
407-
return err
408+
if user_model.IsUserBlockedBy(ctx, doer, newOwner.ID) {
409+
return user_model.ErrBlockedUser
408410
}
409-
if allowed {
410-
return TransferOwnership(ctx, doer, newOwner, repo, teams)
411+
412+
// If new owner is an org and user can create repos he can transfer directly too
413+
if newOwner.IsOrganization() {
414+
allowed, err := organization.CanCreateOrgRepo(ctx, newOwner.ID, doer.ID)
415+
if err != nil {
416+
return err
417+
}
418+
if allowed {
419+
isTransfer = true
420+
return transferOwnership(ctx, doer, newOwner.Name, repo, teams)
421+
}
411422
}
412-
}
413423

414-
// In case the new owner would not have sufficient access to the repo, give access rights for read
415-
hasAccess, err := access_model.HasAnyUnitAccess(ctx, newOwner.ID, repo)
416-
if err != nil {
417-
return err
418-
}
419-
if !hasAccess {
420-
if err := AddOrUpdateCollaborator(ctx, repo, newOwner, perm.AccessModeRead); err != nil {
424+
// In case the new owner would not have sufficient access to the repo, give access rights for read
425+
hasAccess, err := access_model.HasAnyUnitAccess(ctx, newOwner.ID, repo)
426+
if err != nil {
421427
return err
422428
}
423-
}
429+
if !hasAccess {
430+
if err := AddOrUpdateCollaborator(ctx, repo, newOwner, perm.AccessModeRead); err != nil {
431+
return err
432+
}
433+
}
424434

425-
// Make repo as pending for transfer
426-
repo.Status = repo_model.RepositoryPendingTransfer
427-
if err := repo_model.CreatePendingRepositoryTransfer(ctx, doer, newOwner, repo.ID, teams); err != nil {
435+
// Make repo as pending for transfer
436+
repo.Status = repo_model.RepositoryPendingTransfer
437+
return repo_model.CreatePendingRepositoryTransfer(ctx, doer, newOwner, repo.ID, teams)
438+
}); err != nil {
428439
return err
429440
}
430441

431-
// notify users who are able to accept / reject transfer
432-
notify_service.RepoPendingTransfer(ctx, doer, newOwner, repo)
442+
if isTransfer {
443+
notify_service.TransferRepository(ctx, doer, repo, newOwner.Name)
444+
} else {
445+
// notify users who are able to accept / reject transfer
446+
notify_service.RepoPendingTransfer(ctx, doer, newOwner, repo)
447+
}
433448

434449
return nil
435450
}
436451

437-
// CancelRepositoryTransfer marks the repository as ready and remove pending transfer entry,
452+
// RejectRepositoryTransfer marks the repository as ready and remove pending transfer entry,
438453
// thus cancel the transfer process.
439-
func CancelRepositoryTransfer(ctx context.Context, repo *repo_model.Repository) error {
440-
ctx, committer, err := db.TxContext(ctx)
441-
if err != nil {
442-
return err
443-
}
444-
defer committer.Close()
454+
func RejectRepositoryTransfer(ctx context.Context, repo *repo_model.Repository, doer *user_model.User) error {
455+
return db.WithTx(ctx, func(ctx context.Context) error {
456+
repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, repo)
457+
if err != nil {
458+
return err
459+
}
445460

446-
repo.Status = repo_model.RepositoryReady
447-
if err := repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil {
448-
return err
449-
}
461+
if err := repoTransfer.LoadAttributes(ctx); err != nil {
462+
return err
463+
}
450464

451-
if err := repo_model.DeleteRepositoryTransfer(ctx, repo.ID); err != nil {
452-
return err
453-
}
465+
if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) {
466+
return util.ErrPermissionDenied
467+
}
454468

455-
return committer.Commit()
469+
repo.Status = repo_model.RepositoryReady
470+
if err := repo_model.UpdateRepositoryCols(ctx, repo, "status"); err != nil {
471+
return err
472+
}
473+
474+
return repo_model.DeleteRepositoryTransfer(ctx, repo.ID)
475+
})
456476
}

0 commit comments

Comments
 (0)