From b2a9ea0a91ec37a1433506bad31d4ff1adc57aa9 Mon Sep 17 00:00:00 2001 From: Lauris BH Date: Fri, 8 Sep 2017 09:54:35 +0300 Subject: [PATCH] Optimize release creation and update Minimize posibility of race conditions --- models/release.go | 20 +++++----- routers/api/v1/repo/release.go | 56 ++++++++++++++++++--------- routers/repo/release.go | 71 ++++++++++++++-------------------- 3 files changed, 76 insertions(+), 71 deletions(-) diff --git a/models/release.go b/models/release.go index 39c86d54bf44..58c0321f9706 100644 --- a/models/release.go +++ b/models/release.go @@ -140,17 +140,17 @@ func createTag(gitRepo *git.Repository, rel *Release) error { } return err } - } else { - commit, err := gitRepo.GetTagCommit(rel.TagName) - if err != nil { - return fmt.Errorf("GetTagCommit: %v", err) - } + } + commit, err := gitRepo.GetTagCommit(rel.TagName) + if err != nil { + return fmt.Errorf("GetTagCommit: %v", err) + } - rel.Sha1 = commit.ID.String() - rel.NumCommits, err = commit.CommitsCount() - if err != nil { - return fmt.Errorf("CommitsCount: %v", err) - } + rel.Sha1 = commit.ID.String() + rel.CreatedUnix = commit.Author.When.Unix() + rel.NumCommits, err = commit.CommitsCount() + if err != nil { + return fmt.Errorf("CommitsCount: %v", err) } } return nil diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go index 9268bf9cf571..78289a6a2aa4 100644 --- a/routers/api/v1/repo/release.go +++ b/routers/api/v1/repo/release.go @@ -63,29 +63,47 @@ func CreateRelease(ctx *context.APIContext, form api.CreateReleaseOption) { } rel, err := models.GetRelease(ctx.Repo.Repository.ID, form.TagName) if err != nil { - if models.IsErrReleaseNotExist(err) { - ctx.Status(404) + if !models.IsErrReleaseNotExist(err) { + ctx.Handle(500, "GetRelease", err) + return + } + rel = &models.Release{ + RepoID: ctx.Repo.Repository.ID, + PublisherID: ctx.User.ID, + Publisher: ctx.User, + TagName: form.TagName, + Target: form.Target, + Title: form.Title, + Note: form.Note, + IsDraft: form.IsDraft, + IsPrerelease: form.IsPrerelease, + IsTag: false, + } + if err := models.CreateRelease(ctx.Repo.GitRepo, rel, nil); err != nil { + if models.IsErrReleaseAlreadyExist(err) { + ctx.Status(409) + } else { + ctx.Error(500, "CreateRelease", err) + } + return + } + } else { + if !rel.IsTag { + ctx.Status(409) return } - ctx.Handle(500, "GetRelease", err) - return - } - - if !rel.IsTag { - ctx.Status(409) - return - } - rel.Title = form.Title - rel.Note = form.Note - rel.IsDraft = form.IsDraft - rel.IsPrerelease = form.IsPrerelease - rel.PublisherID = ctx.User.ID - rel.IsTag = false + rel.Title = form.Title + rel.Note = form.Note + rel.IsDraft = form.IsDraft + rel.IsPrerelease = form.IsPrerelease + rel.PublisherID = ctx.User.ID + rel.IsTag = false - if err = models.UpdateRelease(ctx.Repo.GitRepo, rel, nil); err != nil { - ctx.Handle(500, "UpdateRelease", err) - return + if err = models.UpdateRelease(ctx.Repo.GitRepo, rel, nil); err != nil { + ctx.Handle(500, "UpdateRelease", err) + return + } } ctx.JSON(201, rel.APIFormat()) } diff --git a/routers/repo/release.go b/routers/repo/release.go index 74bdc57f5b78..fe68f1b6f139 100644 --- a/routers/repo/release.go +++ b/routers/repo/release.go @@ -146,64 +146,28 @@ func NewReleasePost(ctx *context.Context, form auth.NewReleaseForm) { return } - var tagCreatedUnix int64 - tag, err := ctx.Repo.GitRepo.GetTag(form.TagName) - if err == nil { - commit, err := tag.Commit() - if err == nil { - tagCreatedUnix = commit.Author.When.Unix() - } - } - - commit, err := ctx.Repo.GitRepo.GetBranchCommit(form.Target) - if err != nil { - ctx.Handle(500, "GetBranchCommit", err) - return - } - - commitsCount, err := commit.CommitsCount() - if err != nil { - ctx.Handle(500, "CommitsCount", err) - return - } - - rel, err := models.GetRelease(ctx.Repo.Repository.ID, form.TagName) - if err != nil && !models.IsErrReleaseNotExist(err) { - ctx.Handle(500, "GetRelease", err) - return - } - var attachmentUUIDs []string if setting.AttachmentEnabled { attachmentUUIDs = form.Files } - if rel != nil && rel.IsTag { - rel.Title = form.Title - rel.Note = form.Content - rel.IsDraft = len(form.Draft) > 0 - rel.IsPrerelease = form.Prerelease - rel.PublisherID = ctx.User.ID - rel.IsTag = false - - if err = models.UpdateRelease(ctx.Repo.GitRepo, rel, attachmentUUIDs); err != nil { - ctx.Data["Err_TagName"] = true - ctx.Handle(500, "UpdateRelease", err) + rel, err := models.GetRelease(ctx.Repo.Repository.ID, form.TagName) + if err != nil { + if !models.IsErrReleaseNotExist(err) { + ctx.Handle(500, "GetRelease", err) return } - } else { + rel := &models.Release{ RepoID: ctx.Repo.Repository.ID, PublisherID: ctx.User.ID, Title: form.Title, TagName: form.TagName, Target: form.Target, - Sha1: commit.ID.String(), - NumCommits: commitsCount, Note: form.Content, IsDraft: len(form.Draft) > 0, IsPrerelease: form.Prerelease, - CreatedUnix: tagCreatedUnix, + IsTag: false, } if err = models.CreateRelease(ctx.Repo.GitRepo, rel, attachmentUUIDs); err != nil { @@ -218,6 +182,25 @@ func NewReleasePost(ctx *context.Context, form auth.NewReleaseForm) { } return } + } else { + if !rel.IsTag { + ctx.Data["Err_TagName"] = true + ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form) + return + } + + rel.Title = form.Title + rel.Note = form.Content + rel.IsDraft = len(form.Draft) > 0 + rel.IsPrerelease = form.Prerelease + rel.PublisherID = ctx.User.ID + rel.IsTag = false + + if err = models.UpdateRelease(ctx.Repo.GitRepo, rel, attachmentUUIDs); err != nil { + ctx.Data["Err_TagName"] = true + ctx.Handle(500, "UpdateRelease", err) + return + } } log.Trace("Release created: %s/%s:%s", ctx.User.LowerName, ctx.Repo.Repository.Name, form.TagName) @@ -268,6 +251,10 @@ func EditReleasePost(ctx *context.Context, form auth.EditReleaseForm) { } return } + if rel.IsTag { + ctx.Handle(404, "GetRelease", err) + return + } ctx.Data["tag_name"] = rel.TagName ctx.Data["tag_target"] = rel.Target ctx.Data["title"] = rel.Title