From c19ca44d2dbff3bd5c08daa0dae8d479c58edfb4 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 2 Jun 2022 10:31:31 +0800 Subject: [PATCH 1/5] improvement some release related code --- models/release.go | 30 ++++++----------------- models/repo.go | 2 +- routers/api/v1/repo/release.go | 19 +++++++------- routers/api/v1/repo/release_attachment.go | 4 +-- services/release/release.go | 16 +++++------- services/release/release_test.go | 8 +++--- services/repository/push.go | 2 +- 7 files changed, 32 insertions(+), 49 deletions(-) diff --git a/models/release.go b/models/release.go index c7e8bff83ffc5..1b56b5738808c 100644 --- a/models/release.go +++ b/models/release.go @@ -55,7 +55,7 @@ func init() { func (r *Release) loadAttributes(ctx context.Context) error { var err error if r.Repo == nil { - r.Repo, err = repo_model.GetRepositoryByID(r.RepoID) + r.Repo, err = repo_model.GetRepositoryByIDCtx(ctx, r.RepoID) if err != nil { return err } @@ -99,24 +99,12 @@ func (r *Release) HTMLURL() string { } // IsReleaseExist returns true if release with given tag name already exists. -func IsReleaseExist(repoID int64, tagName string) (bool, error) { +func IsReleaseExist(ctx context.Context, repoID int64, tagName string) (bool, error) { if len(tagName) == 0 { return false, nil } - return db.GetEngine(db.DefaultContext).Get(&Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)}) -} - -// InsertRelease inserts a release -func InsertRelease(rel *Release) error { - _, err := db.GetEngine(db.DefaultContext).Insert(rel) - return err -} - -// InsertReleasesContext insert releases -func InsertReleasesContext(ctx context.Context, rels []*Release) error { - _, err := db.GetEngine(ctx).Insert(rels) - return err + return db.GetEngine(ctx).Get(&Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)}) } // UpdateRelease updates all columns of a release @@ -149,22 +137,20 @@ func AddReleaseAttachments(ctx context.Context, releaseID int64, attachmentUUIDs // GetRelease returns release by given ID. func GetRelease(repoID int64, tagName string) (*Release, error) { - isExist, err := IsReleaseExist(repoID, tagName) + rel := &Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)} + isExist, err := db.GetEngine(db.DefaultContext).Get(rel) if err != nil { return nil, err } else if !isExist { return nil, ErrReleaseNotExist{0, tagName} } - - rel := &Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)} - _, err = db.GetEngine(db.DefaultContext).Get(rel) - return rel, err + return rel, nil } // GetReleaseByID returns release with given ID. -func GetReleaseByID(id int64) (*Release, error) { +func GetReleaseByID(ctx context.Context, id int64) (*Release, error) { rel := new(Release) - has, err := db.GetEngine(db.DefaultContext). + has, err := db.GetEngine(ctx). ID(id). Get(rel) if err != nil { diff --git a/models/repo.go b/models/repo.go index d2ad560948b0c..eda80d6d3c290 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1144,7 +1144,7 @@ func LinkedRepository(a *repo_model.Attachment) (*repo_model.Repository, unit.Ty } return repo, unitType, err } else if a.ReleaseID != 0 { - rel, err := GetReleaseByID(a.ReleaseID) + rel, err := GetReleaseByID(db.DefaultContext, a.ReleaseID) if err != nil { return nil, unit.TypeReleases, err } diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go index 7d23a38add22a..e454b418bb182 100644 --- a/routers/api/v1/repo/release.go +++ b/routers/api/v1/repo/release.go @@ -15,7 +15,7 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" - releaseservice "code.gitea.io/gitea/services/release" + release_service "code.gitea.io/gitea/services/release" ) // GetRelease get a single release of a repository @@ -49,7 +49,7 @@ func GetRelease(ctx *context.APIContext) { // "$ref": "#/responses/notFound" id := ctx.ParamsInt64(":id") - release, err := models.GetReleaseByID(id) + release, err := models.GetReleaseByID(ctx, id) if err != nil && !models.IsErrReleaseNotExist(err) { ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err) return @@ -202,7 +202,7 @@ func CreateRelease(ctx *context.APIContext) { IsTag: false, Repo: ctx.Repo.Repository, } - if err := releaseservice.CreateRelease(ctx.Repo.GitRepo, rel, nil, ""); err != nil { + if err := release_service.CreateRelease(ctx.Repo.GitRepo, rel, nil, ""); err != nil { if models.IsErrReleaseAlreadyExist(err) { ctx.Error(http.StatusConflict, "ReleaseAlreadyExist", err) } else { @@ -225,7 +225,7 @@ func CreateRelease(ctx *context.APIContext) { rel.Repo = ctx.Repo.Repository rel.Publisher = ctx.Doer - if err = releaseservice.UpdateRelease(ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil); err != nil { + if err = release_service.UpdateRelease(ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateRelease", err) return } @@ -271,7 +271,7 @@ func EditRelease(ctx *context.APIContext) { form := web.GetForm(ctx).(*api.EditReleaseOption) id := ctx.ParamsInt64(":id") - rel, err := models.GetReleaseByID(id) + rel, err := models.GetReleaseByID(ctx, id) if err != nil && !models.IsErrReleaseNotExist(err) { ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err) return @@ -300,12 +300,13 @@ func EditRelease(ctx *context.APIContext) { if form.IsPrerelease != nil { rel.IsPrerelease = *form.IsPrerelease } - if err := releaseservice.UpdateRelease(ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil); err != nil { + if err := release_service.UpdateRelease(ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil); err != nil { ctx.Error(http.StatusInternalServerError, "UpdateRelease", err) return } - rel, err = models.GetReleaseByID(id) + // reload data from database + rel, err = models.GetReleaseByID(ctx, id) if err != nil { ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err) return @@ -346,7 +347,7 @@ func DeleteRelease(ctx *context.APIContext) { // "$ref": "#/responses/notFound" id := ctx.ParamsInt64(":id") - rel, err := models.GetReleaseByID(id) + rel, err := models.GetReleaseByID(ctx, id) if err != nil && !models.IsErrReleaseNotExist(err) { ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err) return @@ -356,7 +357,7 @@ func DeleteRelease(ctx *context.APIContext) { ctx.NotFound() return } - if err := releaseservice.DeleteReleaseByID(ctx, id, ctx.Doer, false); err != nil { + if err := release_service.DeleteReleaseByID(ctx, id, ctx.Doer, false); err != nil { ctx.Error(http.StatusInternalServerError, "DeleteReleaseByID", err) return } diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index b7807e5e8be88..8694653c06059 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -98,7 +98,7 @@ func ListReleaseAttachments(ctx *context.APIContext) { // "$ref": "#/responses/AttachmentList" releaseID := ctx.ParamsInt64(":id") - release, err := models.GetReleaseByID(releaseID) + release, err := models.GetReleaseByID(ctx, releaseID) if err != nil { ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err) return @@ -164,7 +164,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) { // Check if release exists an load release releaseID := ctx.ParamsInt64(":id") - release, err := models.GetReleaseByID(releaseID) + release, err := models.GetReleaseByID(ctx, releaseID) if err != nil { ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err) return diff --git a/services/release/release.go b/services/release/release.go index 0372e3a6906a0..aee15b5380214 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -112,7 +112,7 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) (bool, // CreateRelease creates a new release of repository. func CreateRelease(gitRepo *git.Repository, rel *models.Release, attachmentUUIDs []string, msg string) error { - isExist, err := models.IsReleaseExist(rel.RepoID, rel.TagName) + isExist, err := models.IsReleaseExist(gitRepo.Ctx, rel.RepoID, rel.TagName) if err != nil { return err } else if isExist { @@ -126,7 +126,7 @@ func CreateRelease(gitRepo *git.Repository, rel *models.Release, attachmentUUIDs } rel.LowerTagName = strings.ToLower(rel.TagName) - if err = models.InsertRelease(rel); err != nil { + if err = db.Insert(gitRepo.Ctx, rel); err != nil { return err } @@ -143,7 +143,7 @@ func CreateRelease(gitRepo *git.Repository, rel *models.Release, attachmentUUIDs // CreateNewTag creates a new repository tag func CreateNewTag(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, commit, tagName, msg string) error { - isExist, err := models.IsReleaseExist(repo.ID, tagName) + isExist, err := models.IsReleaseExist(ctx, repo.ID, tagName) if err != nil { return err } else if isExist { @@ -174,11 +174,7 @@ func CreateNewTag(ctx context.Context, doer *user_model.User, repo *repo_model.R return err } - if err = models.InsertRelease(rel); err != nil { - return err - } - - return err + return db.Insert(ctx, rel) } // UpdateRelease updates information, attachments of a release and will create tag if it's not a draft and tag not exist. @@ -286,12 +282,12 @@ func UpdateRelease(doer *user_model.User, gitRepo *git.Repository, rel *models.R // DeleteReleaseByID deletes a release and corresponding Git tag by given ID. func DeleteReleaseByID(ctx context.Context, id int64, doer *user_model.User, delTag bool) error { - rel, err := models.GetReleaseByID(id) + rel, err := models.GetReleaseByID(ctx, id) if err != nil { return fmt.Errorf("GetReleaseByID: %v", err) } - repo, err := repo_model.GetRepositoryByID(rel.RepoID) + repo, err := repo_model.GetRepositoryByIDCtx(ctx, rel.RepoID) if err != nil { return fmt.Errorf("GetRepositoryByID: %v", err) } diff --git a/services/release/release_test.go b/services/release/release_test.go index 823560a092ce7..0f5b74f70d576 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -162,7 +162,7 @@ func TestRelease_Update(t *testing.T) { time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp release.Note = "Changed note" assert.NoError(t, UpdateRelease(user, gitRepo, release, nil, nil, nil)) - release, err = models.GetReleaseByID(release.ID) + release, err = models.GetReleaseByID(db.DefaultContext, release.ID) assert.NoError(t, err) assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix)) @@ -186,7 +186,7 @@ func TestRelease_Update(t *testing.T) { time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp release.Title = "Changed title" assert.NoError(t, UpdateRelease(user, gitRepo, release, nil, nil, nil)) - release, err = models.GetReleaseByID(release.ID) + release, err = models.GetReleaseByID(db.DefaultContext, release.ID) assert.NoError(t, err) assert.Less(t, int64(releaseCreatedUnix), int64(release.CreatedUnix)) @@ -211,7 +211,7 @@ func TestRelease_Update(t *testing.T) { release.Title = "Changed title" release.Note = "Changed note" assert.NoError(t, UpdateRelease(user, gitRepo, release, nil, nil, nil)) - release, err = models.GetReleaseByID(release.ID) + release, err = models.GetReleaseByID(db.DefaultContext, release.ID) assert.NoError(t, err) assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix)) @@ -236,7 +236,7 @@ func TestRelease_Update(t *testing.T) { tagName := release.TagName assert.NoError(t, UpdateRelease(user, gitRepo, release, nil, nil, nil)) - release, err = models.GetReleaseByID(release.ID) + release, err = models.GetReleaseByID(db.DefaultContext, release.ID) assert.NoError(t, err) assert.Equal(t, tagName, release.TagName) diff --git a/services/repository/push.go b/services/repository/push.go index 5ca8c73983664..5e48a19ba80b6 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -399,7 +399,7 @@ func pushUpdateAddTags(ctx context.Context, repo *repo_model.Repository, gitRepo } if len(newReleases) > 0 { - if err = models.InsertReleasesContext(ctx, newReleases); err != nil { + if err = db.Insert(ctx, newReleases); err != nil { return fmt.Errorf("Insert: %v", err) } } From 890dcba674e46469cbd2e933fd6c7403b71ca2e3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 2 Jun 2022 10:35:00 +0800 Subject: [PATCH 2/5] Some improvements --- models/release.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/release.go b/models/release.go index 1b56b5738808c..b9e51dc9adc3f 100644 --- a/models/release.go +++ b/models/release.go @@ -104,7 +104,7 @@ func IsReleaseExist(ctx context.Context, repoID int64, tagName string) (bool, er return false, nil } - return db.GetEngine(ctx).Get(&Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)}) + return db.GetEngine(ctx).Exist(&Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)}) } // UpdateRelease updates all columns of a release From 6b0c87b44a0d8293a3a355488673a37a034f7ed1 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Thu, 2 Jun 2022 21:38:42 -0400 Subject: [PATCH 3/5] Update models/release.go Co-authored-by: delvh --- models/release.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/release.go b/models/release.go index b9e51dc9adc3f..94e803c716193 100644 --- a/models/release.go +++ b/models/release.go @@ -138,10 +138,10 @@ func AddReleaseAttachments(ctx context.Context, releaseID int64, attachmentUUIDs // GetRelease returns release by given ID. func GetRelease(repoID int64, tagName string) (*Release, error) { rel := &Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)} - isExist, err := db.GetEngine(db.DefaultContext).Get(rel) + has, err := db.GetEngine(db.DefaultContext).Get(rel) if err != nil { return nil, err - } else if !isExist { + } else if !has { return nil, ErrReleaseNotExist{0, tagName} } return rel, nil From 22a38550f2b88aec21210a0df067391723fc8c4c Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Thu, 2 Jun 2022 21:38:47 -0400 Subject: [PATCH 4/5] Update services/release/release.go Co-authored-by: delvh --- services/release/release.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/release/release.go b/services/release/release.go index aee15b5380214..2640e5e95f9c8 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -143,10 +143,10 @@ func CreateRelease(gitRepo *git.Repository, rel *models.Release, attachmentUUIDs // CreateNewTag creates a new repository tag func CreateNewTag(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, commit, tagName, msg string) error { - isExist, err := models.IsReleaseExist(ctx, repo.ID, tagName) + has, err := models.IsReleaseExist(ctx, repo.ID, tagName) if err != nil { return err - } else if isExist { + } else if has { return models.ErrTagAlreadyExists{ TagName: tagName, } From 0b9e8edc3b6ee53a21d4a53a15dad5832add6fcc Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Thu, 2 Jun 2022 21:38:53 -0400 Subject: [PATCH 5/5] Update services/release/release.go Co-authored-by: delvh --- services/release/release.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/release/release.go b/services/release/release.go index 2640e5e95f9c8..b2cbceb12d408 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -112,10 +112,10 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) (bool, // CreateRelease creates a new release of repository. func CreateRelease(gitRepo *git.Repository, rel *models.Release, attachmentUUIDs []string, msg string) error { - isExist, err := models.IsReleaseExist(gitRepo.Ctx, rel.RepoID, rel.TagName) + has, err := models.IsReleaseExist(gitRepo.Ctx, rel.RepoID, rel.TagName) if err != nil { return err - } else if isExist { + } else if has { return models.ErrReleaseAlreadyExist{ TagName: rel.TagName, }