Skip to content

Commit f02138a

Browse files
lunnytechknowlogick
authored andcommitted
Fix bug when migrate from API (#8631)
* fix bug when migrate from API * fix test * fix test * improve * fix error message
1 parent 55bdc9a commit f02138a

File tree

4 files changed

+54
-9
lines changed

4 files changed

+54
-9
lines changed

integrations/api_repo_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ func testAPIRepoMigrateConflict(t *testing.T, u *url.URL) {
334334
resp := httpContext.Session.MakeRequest(t, req, http.StatusConflict)
335335
respJSON := map[string]string{}
336336
DecodeJSON(t, resp, &respJSON)
337-
assert.Equal(t, respJSON["message"], "The repository with the same name already exists.")
337+
assert.Equal(t, "The repository with the same name already exists.", respJSON["message"])
338338
})
339339
}
340340

models/repo.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2840,3 +2840,9 @@ func (repo *Repository) GetTreePathLock(treePath string) (*LFSLock, error) {
28402840
}
28412841
return nil, nil
28422842
}
2843+
2844+
// UpdateRepositoryCols updates repository's columns
2845+
func UpdateRepositoryCols(repo *Repository, cols ...string) error {
2846+
_, err := x.ID(repo.ID).Cols(cols...).Update(repo)
2847+
return err
2848+
}

modules/task/migrate.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,6 @@ func runMigrateTask(t *models.Task) (err error) {
9797
opts.MigrateToRepoID = t.RepoID
9898
repo, err := migrations.MigrateRepository(t.Doer, t.Owner.Name, *opts)
9999
if err == nil {
100-
notification.NotifyMigrateRepository(t.Doer, t.Owner, repo)
101-
102100
log.Trace("Repository migrated [%d]: %s/%s", repo.ID, t.Owner.Name, repo.Name)
103101
return nil
104102
}

routers/api/v1/repo/repo.go

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
package repo
77

88
import (
9+
"bytes"
10+
"errors"
911
"fmt"
1012
"net/http"
1113
"net/url"
@@ -425,15 +427,54 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {
425427
opts.Releases = false
426428
}
427429

428-
repo, err := migrations.MigrateRepository(ctx.User, ctxUser.Name, opts)
429-
if err == nil {
430-
notification.NotifyMigrateRepository(ctx.User, ctxUser, repo)
430+
repo, err := models.CreateRepository(ctx.User, ctxUser, models.CreateRepoOptions{
431+
Name: opts.RepoName,
432+
Description: opts.Description,
433+
OriginalURL: opts.CloneAddr,
434+
IsPrivate: opts.Private,
435+
IsMirror: opts.Mirror,
436+
Status: models.RepositoryBeingMigrated,
437+
})
438+
if err != nil {
439+
handleMigrateError(ctx, ctxUser, remoteAddr, err)
440+
return
441+
}
442+
443+
opts.MigrateToRepoID = repo.ID
431444

432-
log.Trace("Repository migrated: %s/%s", ctxUser.Name, form.RepoName)
433-
ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin))
445+
defer func() {
446+
if e := recover(); e != nil {
447+
var buf bytes.Buffer
448+
fmt.Fprintf(&buf, "Handler crashed with error: %v", log.Stack(2))
449+
450+
err = errors.New(buf.String())
451+
}
452+
453+
if err == nil {
454+
repo.Status = models.RepositoryReady
455+
if err := models.UpdateRepositoryCols(repo, "status"); err == nil {
456+
notification.NotifyMigrateRepository(ctx.User, ctxUser, repo)
457+
return
458+
}
459+
}
460+
461+
if repo != nil {
462+
if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil {
463+
log.Error("DeleteRepository: %v", errDelete)
464+
}
465+
}
466+
}()
467+
468+
if _, err = migrations.MigrateRepository(ctx.User, ctxUser.Name, opts); err != nil {
469+
handleMigrateError(ctx, ctxUser, remoteAddr, err)
434470
return
435471
}
436472

473+
log.Trace("Repository migrated: %s/%s", ctxUser.Name, form.RepoName)
474+
ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin))
475+
}
476+
477+
func handleMigrateError(ctx *context.APIContext, repoOwner *models.User, remoteAddr string, err error) {
437478
switch {
438479
case models.IsErrRepoAlreadyExist(err):
439480
ctx.Error(409, "", "The repository with the same name already exists.")
@@ -442,7 +483,7 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {
442483
case migrations.IsTwoFactorAuthError(err):
443484
ctx.Error(422, "", "Remote visit required two factors authentication.")
444485
case models.IsErrReachLimitOfRepo(err):
445-
ctx.Error(422, "", fmt.Sprintf("You have already reached your limit of %d repositories.", ctxUser.MaxCreationLimit()))
486+
ctx.Error(422, "", fmt.Sprintf("You have already reached your limit of %d repositories.", repoOwner.MaxCreationLimit()))
446487
case models.IsErrNameReserved(err):
447488
ctx.Error(422, "", fmt.Sprintf("The username '%s' is reserved.", err.(models.ErrNameReserved).Name))
448489
case models.IsErrNamePatternNotAllowed(err):

0 commit comments

Comments
 (0)