Skip to content

Commit 926b25b

Browse files
lunnylunny
committed
Remove the unnecessary parameter isCreate from UpdateReleaseOrCreatReleaseFromTag
1 parent 26323d2 commit 926b25b

File tree

2 files changed

+71
-30
lines changed

2 files changed

+71
-30
lines changed

‎services/release/release.go

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,39 +18,41 @@ import (
1818
"code.gitea.io/gitea/modules/timeutil"
1919
)
2020

21-
func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
21+
func createTag(gitRepo *git.Repository, rel *models.Release, msg string) (bool, error) {
22+
var created bool
2223
// Only actual create when publish.
2324
if !rel.IsDraft {
2425
if !gitRepo.IsTagExist(rel.TagName) {
2526
commit, err := gitRepo.GetCommit(rel.Target)
2627
if err != nil {
27-
return fmt.Errorf("GetCommit: %v", err)
28+
return false, fmt.Errorf("GetCommit: %v", err)
2829
}
2930

3031
// Trim '--' prefix to prevent command line argument vulnerability.
3132
rel.TagName = strings.TrimPrefix(rel.TagName, "--")
3233
if len(msg) > 0 {
3334
if err = gitRepo.CreateAnnotatedTag(rel.TagName, msg, commit.ID.String()); err != nil {
3435
if strings.Contains(err.Error(), "is not a valid tag name") {
35-
return models.ErrInvalidTagName{
36+
return false, models.ErrInvalidTagName{
3637
TagName: rel.TagName,
3738
}
3839
}
39-
return err
40+
return false, err
4041
}
4142
} else if err = gitRepo.CreateTag(rel.TagName, commit.ID.String()); err != nil {
4243
if strings.Contains(err.Error(), "is not a valid tag name") {
43-
return models.ErrInvalidTagName{
44+
return false, models.ErrInvalidTagName{
4445
TagName: rel.TagName,
4546
}
4647
}
47-
return err
48+
return false, err
4849
}
50+
created = true
4951
rel.LowerTagName = strings.ToLower(rel.TagName)
5052
// Prepare Notify
5153
if err := rel.LoadAttributes(); err != nil {
5254
log.Error("LoadAttributes: %v", err)
53-
return err
55+
return false, err
5456
}
5557
notification.NotifyPushCommits(
5658
rel.Publisher, rel.Repo,
@@ -64,13 +66,13 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
6466
}
6567
commit, err := gitRepo.GetTagCommit(rel.TagName)
6668
if err != nil {
67-
return fmt.Errorf("GetTagCommit: %v", err)
69+
return false, fmt.Errorf("GetTagCommit: %v", err)
6870
}
6971

7072
rel.Sha1 = commit.ID.String()
7173
rel.NumCommits, err = commit.CommitsCount()
7274
if err != nil {
73-
return fmt.Errorf("CommitsCount: %v", err)
75+
return false, fmt.Errorf("CommitsCount: %v", err)
7476
}
7577

7678
if rel.PublisherID <= 0 {
@@ -79,11 +81,10 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
7981
rel.PublisherID = u.ID
8082
}
8183
}
82-
8384
} else {
8485
rel.CreatedUnix = timeutil.TimeStampNow()
8586
}
86-
return nil
87+
return created, nil
8788
}
8889

8990
// CreateRelease creates a new release of repository.
@@ -97,7 +98,7 @@ func CreateRelease(gitRepo *git.Repository, rel *models.Release, attachmentUUIDs
9798
}
9899
}
99100

100-
if err = createTag(gitRepo, rel, msg); err != nil {
101+
if _, err = createTag(gitRepo, rel, msg); err != nil {
101102
return err
102103
}
103104

@@ -144,7 +145,7 @@ func CreateNewTag(doer *models.User, repo *models.Repository, commit, tagName, m
144145
IsTag: true,
145146
}
146147

147-
if err = createTag(gitRepo, rel, msg); err != nil {
148+
if _, err = createTag(gitRepo, rel, msg); err != nil {
148149
return err
149150
}
150151

@@ -156,9 +157,16 @@ func CreateNewTag(doer *models.User, repo *models.Repository, commit, tagName, m
156157
}
157158

158159
// UpdateReleaseOrCreatReleaseFromTag updates information of a release or create release from tag.
160+
// addAttachmentUUIDs accept a slice of new created attachments' uuids which will be reassigned release_id as the created release
161+
// delAttachmentUUIDs accept a slice of attachments' uuids which will be deleted from the release
162+
// editAttachments accept a map of attachment uuid to new attachment name which will be updated with attachments.
159163
func UpdateReleaseOrCreatReleaseFromTag(doer *models.User, gitRepo *git.Repository, rel *models.Release,
160-
addAttachmentUUIDs, delAttachmentUUIDs []string, editAttachments map[string]string, isCreate bool) (err error) {
161-
if err = createTag(gitRepo, rel, ""); err != nil {
164+
addAttachmentUUIDs, delAttachmentUUIDs []string, editAttachments map[string]string) (err error) {
165+
if rel.ID == 0 {
166+
return errors.New("UpdateReleaseOrCreatReleaseFromTag only accepts an exist release")
167+
}
168+
isCreated, err := createTag(gitRepo, rel, "")
169+
if err != nil {
162170
return err
163171
}
164172
rel.LowerTagName = strings.ToLower(rel.TagName)
@@ -230,11 +238,15 @@ func UpdateReleaseOrCreatReleaseFromTag(doer *models.User, gitRepo *git.Reposito
230238

231239
for _, uuid := range delAttachmentUUIDs {
232240
if err := storage.Attachments.Delete(models.AttachmentRelativePath(uuid)); err != nil {
233-
return err
241+
// Even delete files failed, but the attachments has been removed from database, so we
242+
// should not return error but only record the error on logs.
243+
// users have to delete this attachments manually or we should have a
244+
// synchronize between database attachment table and attachment storage
245+
log.Error("delete attachment[uuid: %s] failed: %v", uuid, err)
234246
}
235247
}
236248

237-
if !isCreate {
249+
if !isCreated {
238250
notification.NotifyUpdateRelease(doer, rel)
239251
return
240252
}

‎services/release/release_test.go

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func TestRelease_Update(t *testing.T) {
139139
releaseCreatedUnix := release.CreatedUnix
140140
time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
141141
release.Note = "Changed note"
142-
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, nil, nil, false))
142+
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, nil, nil))
143143
release, err = models.GetReleaseByID(release.ID)
144144
assert.NoError(t, err)
145145
assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
@@ -161,7 +161,7 @@ func TestRelease_Update(t *testing.T) {
161161
releaseCreatedUnix = release.CreatedUnix
162162
time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
163163
release.Title = "Changed title"
164-
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, nil, nil, false))
164+
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, nil, nil))
165165
release, err = models.GetReleaseByID(release.ID)
166166
assert.NoError(t, err)
167167
assert.Less(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
@@ -184,19 +184,42 @@ func TestRelease_Update(t *testing.T) {
184184
time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
185185
release.Title = "Changed title"
186186
release.Note = "Changed note"
187-
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, nil, nil, false))
187+
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, nil, nil))
188188
release, err = models.GetReleaseByID(release.ID)
189189
assert.NoError(t, err)
190190
assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
191191

192+
// Test create release
193+
release = &models.Release{
194+
RepoID: repo.ID,
195+
PublisherID: user.ID,
196+
TagName: "v1.1.2",
197+
Target: "master",
198+
Title: "v1.1.2 is released",
199+
Note: "v1.1.2 is released",
200+
IsDraft: true,
201+
IsPrerelease: false,
202+
IsTag: false,
203+
}
204+
assert.NoError(t, CreateRelease(gitRepo, release, nil, ""))
205+
assert.Greater(t, release.ID, int64(0))
206+
207+
release.IsDraft = false
208+
tagName := release.TagName
209+
210+
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, nil, nil))
211+
release, err = models.GetReleaseByID(release.ID)
212+
assert.NoError(t, err)
213+
assert.Equal(t, tagName, release.TagName)
214+
215+
// Add new attachments
192216
attach, err := models.NewAttachment(&models.Attachment{
193217
UploaderID: user.ID,
194218
Name: "test.txt",
195219
}, []byte{}, strings.NewReader("testtest"))
196220
assert.NoError(t, err)
197221

198-
// Add new attachments
199-
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, []string{attach.UUID}, nil, nil, false))
222+
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, []string{attach.UUID}, nil, nil))
200223
assert.NoError(t, models.GetReleaseAttachments(release))
201224
assert.EqualValues(t, 1, len(release.Attachments))
202225
assert.EqualValues(t, attach.UUID, release.Attachments[0].UUID)
@@ -206,7 +229,7 @@ func TestRelease_Update(t *testing.T) {
206229
// update the attachment name
207230
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, nil, map[string]string{
208231
attach.UUID: "test2.txt",
209-
}, false))
232+
}))
210233
release.Attachments = nil
211234
assert.NoError(t, models.GetReleaseAttachments(release))
212235
assert.EqualValues(t, 1, len(release.Attachments))
@@ -215,7 +238,7 @@ func TestRelease_Update(t *testing.T) {
215238
assert.EqualValues(t, "test2.txt", release.Attachments[0].Name)
216239

217240
// delete the attachment
218-
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, []string{attach.UUID}, nil, false))
241+
assert.NoError(t, UpdateReleaseOrCreatReleaseFromTag(user, gitRepo, release, nil, []string{attach.UUID}, nil))
219242
release.Attachments = nil
220243
assert.NoError(t, models.GetReleaseAttachments(release))
221244
assert.EqualValues(t, 0, len(release.Attachments))
@@ -244,12 +267,14 @@ func TestRelease_createTag(t *testing.T) {
244267
IsPrerelease: false,
245268
IsTag: false,
246269
}
247-
assert.NoError(t, createTag(gitRepo, release, ""))
270+
_, err = createTag(gitRepo, release, "")
271+
assert.NoError(t, err)
248272
assert.NotEmpty(t, release.CreatedUnix)
249273
releaseCreatedUnix := release.CreatedUnix
250274
time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
251275
release.Note = "Changed note"
252-
assert.NoError(t, createTag(gitRepo, release, ""))
276+
_, err = createTag(gitRepo, release, "")
277+
assert.NoError(t, err)
253278
assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
254279

255280
// Test a changed draft
@@ -264,11 +289,13 @@ func TestRelease_createTag(t *testing.T) {
264289
IsPrerelease: false,
265290
IsTag: false,
266291
}
267-
assert.NoError(t, createTag(gitRepo, release, ""))
292+
_, err = createTag(gitRepo, release, "")
293+
assert.NoError(t, err)
268294
releaseCreatedUnix = release.CreatedUnix
269295
time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
270296
release.Title = "Changed title"
271-
assert.NoError(t, createTag(gitRepo, release, ""))
297+
_, err = createTag(gitRepo, release, "")
298+
assert.NoError(t, err)
272299
assert.Less(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
273300

274301
// Test a changed pre-release
@@ -283,12 +310,14 @@ func TestRelease_createTag(t *testing.T) {
283310
IsPrerelease: true,
284311
IsTag: false,
285312
}
286-
assert.NoError(t, createTag(gitRepo, release, ""))
313+
_, err = createTag(gitRepo, release, "")
314+
assert.NoError(t, err)
287315
releaseCreatedUnix = release.CreatedUnix
288316
time.Sleep(2 * time.Second) // sleep 2 seconds to ensure a different timestamp
289317
release.Title = "Changed title"
290318
release.Note = "Changed note"
291-
assert.NoError(t, createTag(gitRepo, release, ""))
319+
_, err = createTag(gitRepo, release, "")
320+
assert.NoError(t, err)
292321
assert.Equal(t, int64(releaseCreatedUnix), int64(release.CreatedUnix))
293322
}
294323

0 commit comments

Comments
 (0)