Optimize release creation and update

Minimize posibility of race conditions
go-gitea · Sep 16, 2017 · b2a9ea0 · b2a9ea0
1 parent 8b78ce7
commit b2a9ea0
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 71 deletions.
diff --git 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
@@ -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
@@ -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