From a828151c3b56eff2aad3d4c5ee2a875c24dc6d5a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 23 Oct 2019 00:30:13 +0800 Subject: [PATCH 1/5] fix bug when migrate from API --- models/repo.go | 6 +++++ modules/task/migrate.go | 2 -- routers/api/v1/repo/repo.go | 44 ++++++++++++++++++++++++++++++++++--- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/models/repo.go b/models/repo.go index 7945cb309d341..31bd510d7c6e3 100644 --- a/models/repo.go +++ b/models/repo.go @@ -2826,3 +2826,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 5d15a506d7932..247403d7be355 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 422bb0ac3a2ef..271761478a85f 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,10 +427,46 @@ 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) + 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 { + ctx.Error(500, "CreateRepository", 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) + } + } + }() + + _, err = migrations.MigrateRepository(ctx.User, ctxUser.Name, opts) + if err == nil { log.Trace("Repository migrated: %s/%s", ctxUser.Name, form.RepoName) ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin)) return From f71042b70702f9c81dcf170659e2c25e3ede9d51 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 23 Oct 2019 15:23:42 +0800 Subject: [PATCH 2/5] fix test --- integrations/api_repo_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index 60fe4a3649576..eeb6bba9d7533 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, "repository already exists [uname: user2, name: repo-tmp-17]", respJSON["message"]) }) } From fd1842758b7c9c9d7988cdb5389e8b9bfffdf4b5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 24 Oct 2019 15:41:20 +0800 Subject: [PATCH 3/5] fix test --- integrations/api_repo_test.go | 2 +- routers/api/v1/repo/repo.go | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index eeb6bba9d7533..a2683d4af4762 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, "repository already exists [uname: user2, name: repo-tmp-17]", respJSON["message"]) + assert.Equal(t, "The repository with the same name already exists.", respJSON["message"]) }) } diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 271761478a85f..7fc42b759a6c9 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -436,7 +436,7 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { Status: models.RepositoryBeingMigrated, }) if err != nil { - ctx.Error(500, "CreateRepository", err) + handleMigrateError(ctx, remoteAddr, err) return } @@ -472,6 +472,10 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { return } + handleMigrateError(ctx, remoteAddr, err) +} + +func handleMigrateError(ctx *context.APIContext, remoteAddr string, err error) { switch { case models.IsErrRepoAlreadyExist(err): ctx.Error(409, "", "The repository with the same name already exists.") @@ -480,7 +484,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.", ctx.User.MaxCreationLimit())) case models.IsErrNameReserved(err): ctx.Error(422, "", fmt.Sprintf("The username '%s' is reserved.", err.(models.ErrNameReserved).Name)) case models.IsErrNamePatternNotAllowed(err): From 437e90d8f773766af72dcf33295ab9682f5cec8d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 5 Nov 2019 14:59:20 +0800 Subject: [PATCH 4/5] improve --- routers/api/v1/repo/repo.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 7fc42b759a6c9..3c0fae5c8e163 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -465,14 +465,13 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { } }() - _, err = migrations.MigrateRepository(ctx.User, ctxUser.Name, opts) - if err == nil { - log.Trace("Repository migrated: %s/%s", ctxUser.Name, form.RepoName) - ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin)) + if _, err = migrations.MigrateRepository(ctx.User, ctxUser.Name, opts); err != nil { + handleMigrateError(ctx, remoteAddr, err) return } - handleMigrateError(ctx, remoteAddr, err) + log.Trace("Repository migrated: %s/%s", ctxUser.Name, form.RepoName) + ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin)) } func handleMigrateError(ctx *context.APIContext, remoteAddr string, err error) { From 6348549239d76bd7e794ef0273dcef2a038ba8d9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 5 Nov 2019 16:43:56 +0800 Subject: [PATCH 5/5] fix error message --- routers/api/v1/repo/repo.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 3c0fae5c8e163..a86f6565e6b4e 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -436,7 +436,7 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { Status: models.RepositoryBeingMigrated, }) if err != nil { - handleMigrateError(ctx, remoteAddr, err) + handleMigrateError(ctx, ctxUser, remoteAddr, err) return } @@ -466,7 +466,7 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { }() if _, err = migrations.MigrateRepository(ctx.User, ctxUser.Name, opts); err != nil { - handleMigrateError(ctx, remoteAddr, err) + handleMigrateError(ctx, ctxUser, remoteAddr, err) return } @@ -474,7 +474,7 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { ctx.JSON(201, repo.APIFormat(models.AccessModeAdmin)) } -func handleMigrateError(ctx *context.APIContext, remoteAddr string, err error) { +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.") @@ -483,7 +483,7 @@ func handleMigrateError(ctx *context.APIContext, remoteAddr string, err error) { 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.", ctx.User.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):