From f02138a1480015b9aa6ab5eef82506242d98612e Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Sat, 9 Nov 2019 06:21:00 +0800
Subject: [PATCH] Fix bug when migrate from API (#8631)
* fix bug when migrate from API
* fix test
* fix test
* improve
* fix error message
---
integrations/api_repo_test.go | 2 +-
models/repo.go | 6 ++++
modules/task/migrate.go | 2 --
routers/api/v1/repo/repo.go | 55 ++++++++++++++++++++++++++++++-----
4 files changed, 55 insertions(+), 10 deletions(-)
diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go
index 60fe4a364..a2683d4af 100644
--- a/integrations/api_repo_test.go
+++ b/integrations/api_repo_test.go
@@ -334,7 +334,7 @@ func testAPIRepoMigrateConflict(t *testing.T, u *url.URL) {
resp := httpContext.Session.MakeRequest(t, req, http.StatusConflict)
respJSON := map[string]string{}
DecodeJSON(t, resp, &respJSON)
- assert.Equal(t, respJSON["message"], "The repository with the same name already exists.")
+ assert.Equal(t, "The repository with the same name already exists.", respJSON["message"])
})
}
diff --git a/models/repo.go b/models/repo.go
index e0a93aaeb..812460e92 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -2840,3 +2840,9 @@ func (repo *Repository) GetTreePathLock(treePath string) (*LFSLock, error) {
}
return nil, nil
}
+
+// UpdateRepositoryCols updates repository's columns
+func UpdateRepositoryCols(repo *Repository, cols ...string) error {
+ _, err := x.ID(repo.ID).Cols(cols...).Update(repo)
+ return err
+}
diff --git a/modules/task/migrate.go b/modules/task/migrate.go
index 5d15a506d..247403d7b 100644
--- a/modules/task/migrate.go
+++ b/modules/task/migrate.go
@@ -97,8 +97,6 @@ func runMigrateTask(t *models.Task) (err error) {
opts.MigrateToRepoID = t.RepoID
repo, err := migrations.MigrateRepository(t.Doer, t.Owner.Name, *opts)
if err == nil {
- notification.NotifyMigrateRepository(t.Doer, t.Owner, repo)
-
log.Trace("Repository migrated [%d]: %s/%s", repo.ID, t.Owner.Name, repo.Name)
return nil
}
diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go
index d366435a2..7b752370d 100644
--- a/routers/api/v1/repo/repo.go
+++ b/routers/api/v1/repo/repo.go
@@ -6,6 +6,8 @@
package repo
import (
+ "bytes"
+ "errors"
"fmt"
"net/http"
"net/url"
@@ -425,15 +427,54 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {
opts.Releases = false
}
- repo, err := migrations.MigrateRepository(ctx.User, ctxUser.Name, opts)
- if err == nil {
- notification.NotifyMigrateRepository(ctx.User, ctxUser, repo)
-
- log.Trace("Repository migrated: %s/%s", ctxUser.Name, form.RepoName)
- ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin))
+ repo, err := models.CreateRepository(ctx.User, ctxUser, models.CreateRepoOptions{
+ Name: opts.RepoName,
+ Description: opts.Description,
+ OriginalURL: opts.CloneAddr,
+ IsPrivate: opts.Private,
+ IsMirror: opts.Mirror,
+ Status: models.RepositoryBeingMigrated,
+ })
+ if err != nil {
+ handleMigrateError(ctx, ctxUser, remoteAddr, err)
return
}
+ opts.MigrateToRepoID = repo.ID
+
+ defer func() {
+ if e := recover(); e != nil {
+ var buf bytes.Buffer
+ fmt.Fprintf(&buf, "Handler crashed with error: %v", log.Stack(2))
+
+ err = errors.New(buf.String())
+ }
+
+ if err == nil {
+ repo.Status = models.RepositoryReady
+ if err := models.UpdateRepositoryCols(repo, "status"); err == nil {
+ notification.NotifyMigrateRepository(ctx.User, ctxUser, repo)
+ return
+ }
+ }
+
+ if repo != nil {
+ if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil {
+ log.Error("DeleteRepository: %v", errDelete)
+ }
+ }
+ }()
+
+ if _, err = migrations.MigrateRepository(ctx.User, ctxUser.Name, opts); err != nil {
+ handleMigrateError(ctx, ctxUser, remoteAddr, err)
+ return
+ }
+
+ log.Trace("Repository migrated: %s/%s", ctxUser.Name, form.RepoName)
+ ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin))
+}
+
+func handleMigrateError(ctx *context.APIContext, repoOwner *models.User, remoteAddr string, err error) {
switch {
case models.IsErrRepoAlreadyExist(err):
ctx.Error(409, "", "The repository with the same name already exists.")
@@ -442,7 +483,7 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {
case migrations.IsTwoFactorAuthError(err):
ctx.Error(422, "", "Remote visit required two factors authentication.")
case models.IsErrReachLimitOfRepo(err):
- ctx.Error(422, "", fmt.Sprintf("You have already reached your limit of %d repositories.", ctxUser.MaxCreationLimit()))
+ ctx.Error(422, "", fmt.Sprintf("You have already reached your limit of %d repositories.", repoOwner.MaxCreationLimit()))
case models.IsErrNameReserved(err):
ctx.Error(422, "", fmt.Sprintf("The username '%s' is reserved.", err.(models.ErrNameReserved).Name))
case models.IsErrNamePatternNotAllowed(err):