From f818f9ccab6f7836574bd51d1a662e908fbedd7f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 9 Nov 2019 06:21:00 +0800 Subject: [PATCH 1/3] 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 | 22 ++++++++++++++++++++++ modules/task/migrate.go | 2 -- routers/api/v1/repo/repo.go | 32 +++++++++++++++++++++++++------- 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index 60fe4a3649576..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, 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 582df7613a480..9c83359a14512 100644 --- a/models/repo.go +++ b/models/repo.go @@ -2811,3 +2811,25 @@ func (repo *Repository) GetOriginalURLHostname() string { return u.Host } + +// GetTreePathLock returns LSF lock for the treePath +func (repo *Repository) GetTreePathLock(treePath string) (*LFSLock, error) { + if setting.LFS.StartServer { + locks, err := GetLFSLockByRepoID(repo.ID) + if err != nil { + return nil, err + } + for _, lock := range locks { + if lock.Path == treePath { + return lock, nil + } + } + } + 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 8edb53a61f8ea..44380984060a2 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -433,17 +433,32 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { opts.Releases = false } - var repo *models.Repository + 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.UpdateRepositoryStatus(repo.ID, repo.Status); err == nil { + if err := models.UpdateRepositoryCols(repo, "status"); err == nil { notification.NotifyMigrateRepository(ctx.User, ctxUser, repo) return } @@ -456,13 +471,16 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { } }() - repo, 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, 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.") @@ -471,7 +489,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): From ab7c0c76f8d7422be6df84a3bc3f3c736f256db3 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 31 Dec 2019 09:11:01 +0100 Subject: [PATCH 2/3] Update routers/api/v1/repo/repo.go --- routers/api/v1/repo/repo.go | 1 - 1 file changed, 1 deletion(-) diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 44380984060a2..7eb397b68c2f8 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -452,7 +452,6 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) { 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()) } From 4c12e60b81022400d7514d8305aeb458461811b9 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 1 Jan 2020 15:50:52 +0100 Subject: [PATCH 3/3] remove unrelated --- models/repo.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/models/repo.go b/models/repo.go index 9c83359a14512..dc0b42ee31b7f 100644 --- a/models/repo.go +++ b/models/repo.go @@ -2812,22 +2812,6 @@ func (repo *Repository) GetOriginalURLHostname() string { return u.Host } -// GetTreePathLock returns LSF lock for the treePath -func (repo *Repository) GetTreePathLock(treePath string) (*LFSLock, error) { - if setting.LFS.StartServer { - locks, err := GetLFSLockByRepoID(repo.ID) - if err != nil { - return nil, err - } - for _, lock := range locks { - if lock.Path == treePath { - return lock, nil - } - } - } - return nil, nil -} - // UpdateRepositoryCols updates repository's columns func UpdateRepositoryCols(repo *Repository, cols ...string) error { _, err := x.ID(repo.ID).Cols(cols...).Update(repo)