Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport] Fix bug when migrate from API (#8631) #9563

Merged
merged 4 commits into from
Jan 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion integrations/api_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
})
}

Expand Down
6 changes: 6 additions & 0 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -2811,3 +2811,9 @@ func (repo *Repository) GetOriginalURLHostname() string {

return u.Host
}

// UpdateRepositoryCols updates repository's columns
func UpdateRepositoryCols(repo *Repository, cols ...string) error {
_, err := x.ID(repo.ID).Cols(cols...).Update(repo)
return err
}
2 changes: 0 additions & 2 deletions modules/task/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
31 changes: 24 additions & 7 deletions routers/api/v1/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,21 @@ 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
Expand All @@ -443,7 +457,7 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {

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
}
Expand All @@ -456,13 +470,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.")
Expand All @@ -471,7 +488,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):
Expand Down