Implement delete release attachments and update release attachments' name (#14130)

* Implement delete release attachment

* Add attachments on release edit page

* Fix bug

* Finish del release attachments

* Fix frontend lint

* Fix tests

* Support edit release attachments

* Added tests

* Remove the unnecessary parameter isCreate from UpdateReleaseOrCreatReleaseFromTag

* Rename UpdateReleaseOrCreatReleaseFromTag to UpdateRelease

* Fix middle align

1 files changed, 98 insertions, 21 deletions

diff --git a/services/release/release.go b/services/release/release.go
index d04e538c92..9d201edf6d 100644
--- a/services/release/release.go
+++ b/services/release/release.go
@@ -5,6 +5,7 @@
 package release
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -17,13 +18,14 @@ import (
 	"code.gitea.io/gitea/modules/timeutil"
 )
 
-func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
+func createTag(gitRepo *git.Repository, rel *models.Release, msg string) (bool, error) {
+	var created bool
 	// Only actual create when publish.
 	if !rel.IsDraft {
 		if !gitRepo.IsTagExist(rel.TagName) {
 			commit, err := gitRepo.GetCommit(rel.Target)
 			if err != nil {
-				return fmt.Errorf("GetCommit: %v", err)
+				return false, fmt.Errorf("GetCommit: %v", err)
 			}
 
 			// Trim '--' prefix to prevent command line argument vulnerability.
@@ -31,25 +33,26 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
 			if len(msg) > 0 {
 				if err = gitRepo.CreateAnnotatedTag(rel.TagName, msg, commit.ID.String()); err != nil {
 					if strings.Contains(err.Error(), "is not a valid tag name") {
-						return models.ErrInvalidTagName{
+						return false, models.ErrInvalidTagName{
 							TagName: rel.TagName,
 						}
 					}
-					return err
+					return false, err
 				}
 			} else if err = gitRepo.CreateTag(rel.TagName, commit.ID.String()); err != nil {
 				if strings.Contains(err.Error(), "is not a valid tag name") {
-					return models.ErrInvalidTagName{
+					return false, models.ErrInvalidTagName{
 						TagName: rel.TagName,
 					}
 				}
-				return err
+				return false, err
 			}
+			created = true
 			rel.LowerTagName = strings.ToLower(rel.TagName)
 			// Prepare Notify
 			if err := rel.LoadAttributes(); err != nil {
 				log.Error("LoadAttributes: %v", err)
-				return err
+				return false, err
 			}
 			notification.NotifyPushCommits(
 				rel.Publisher, rel.Repo,
@@ -63,13 +66,13 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
 		}
 		commit, err := gitRepo.GetTagCommit(rel.TagName)
 		if err != nil {
-			return fmt.Errorf("GetTagCommit: %v", err)
+			return false, fmt.Errorf("GetTagCommit: %v", err)
 		}
 
 		rel.Sha1 = commit.ID.String()
 		rel.NumCommits, err = commit.CommitsCount()
 		if err != nil {
-			return fmt.Errorf("CommitsCount: %v", err)
+			return false, fmt.Errorf("CommitsCount: %v", err)
 		}
 
 		if rel.PublisherID <= 0 {
@@ -78,11 +81,10 @@ func createTag(gitRepo *git.Repository, rel *models.Release, msg string) error {
 				rel.PublisherID = u.ID
 			}
 		}
-
 	} else {
 		rel.CreatedUnix = timeutil.TimeStampNow()
 	}
-	return nil
+	return created, nil
 }
 
 // CreateRelease creates a new release of repository.
@@ -96,7 +98,7 @@ func CreateRelease(gitRepo *git.Repository, rel *models.Release, attachmentUUIDs
 		}
 	}
 
-	if err = createTag(gitRepo, rel, msg); err != nil {
+	if _, err = createTag(gitRepo, rel, msg); err != nil {
 		return err
 	}
 
@@ -105,7 +107,7 @@ func CreateRelease(gitRepo *git.Repository, rel *models.Release, attachmentUUIDs
 		return err
 	}
 
-	if err = models.AddReleaseAttachments(rel.ID, attachmentUUIDs); err != nil {
+	if err = models.AddReleaseAttachments(models.DefaultDBContext(), rel.ID, attachmentUUIDs); err != nil {
 		return err
 	}
 
@@ -143,7 +145,7 @@ func CreateNewTag(doer *models.User, repo *models.Repository, commit, tagName, m
 		IsTag:        true,
 	}
 
-	if err = createTag(gitRepo, rel, msg); err != nil {
+	if _, err = createTag(gitRepo, rel, msg); err != nil {
 		return err
 	}
 
@@ -154,22 +156,97 @@ func CreateNewTag(doer *models.User, repo *models.Repository, commit, tagName, m
 	return err
 }
 
-// UpdateReleaseOrCreatReleaseFromTag updates information of a release or create release from tag.
-func UpdateReleaseOrCreatReleaseFromTag(doer *models.User, gitRepo *git.Repository, rel *models.Release, attachmentUUIDs []string, isCreate bool) (err error) {
-	if err = createTag(gitRepo, rel, ""); err != nil {
+// UpdateRelease updates information, attachments of a release and will create tag if it's not a draft and tag not exist.
+// addAttachmentUUIDs accept a slice of new created attachments' uuids which will be reassigned release_id as the created release
+// delAttachmentUUIDs accept a slice of attachments' uuids which will be deleted from the release
+// editAttachments accept a map of attachment uuid to new attachment name which will be updated with attachments.
+func UpdateRelease(doer *models.User, gitRepo *git.Repository, rel *models.Release,
+	addAttachmentUUIDs, delAttachmentUUIDs []string, editAttachments map[string]string) (err error) {
+	if rel.ID == 0 {
+		return errors.New("UpdateRelease only accepts an exist release")
+	}
+	isCreated, err := createTag(gitRepo, rel, "")
+	if err != nil {
 		return err
 	}
 	rel.LowerTagName = strings.ToLower(rel.TagName)
 
-	if err = models.UpdateRelease(models.DefaultDBContext(), rel); err != nil {
+	ctx, commiter, err := models.TxDBContext()
+	if err != nil {
+		return err
+	}
+	defer commiter.Close()
+
+	if err = models.UpdateRelease(ctx, rel); err != nil {
 		return err
 	}
 
-	if err = models.AddReleaseAttachments(rel.ID, attachmentUUIDs); err != nil {
-		log.Error("AddReleaseAttachments: %v", err)
+	if err = models.AddReleaseAttachments(ctx, rel.ID, addAttachmentUUIDs); err != nil {
+		return fmt.Errorf("AddReleaseAttachments: %v", err)
+	}
+
+	var deletedUUIDsMap = make(map[string]bool)
+	if len(delAttachmentUUIDs) > 0 {
+		// Check attachments
+		attachments, err := models.GetAttachmentsByUUIDs(ctx, delAttachmentUUIDs)
+		if err != nil {
+			return fmt.Errorf("GetAttachmentsByUUIDs [uuids: %v]: %v", delAttachmentUUIDs, err)
+		}
+		for _, attach := range attachments {
+			if attach.ReleaseID != rel.ID {
+				return errors.New("delete attachement of release permission denied")
+			}
+			deletedUUIDsMap[attach.UUID] = true
+		}
+
+		if _, err := models.DeleteAttachments(ctx, attachments, false); err != nil {
+			return fmt.Errorf("DeleteAttachments [uuids: %v]: %v", delAttachmentUUIDs, err)
+		}
+	}
+
+	if len(editAttachments) > 0 {
+		var updateAttachmentsList = make([]string, 0, len(editAttachments))
+		for k := range editAttachments {
+			updateAttachmentsList = append(updateAttachmentsList, k)
+		}
+		// Check attachments
+		attachments, err := models.GetAttachmentsByUUIDs(ctx, updateAttachmentsList)
+		if err != nil {
+			return fmt.Errorf("GetAttachmentsByUUIDs [uuids: %v]: %v", updateAttachmentsList, err)
+		}
+		for _, attach := range attachments {
+			if attach.ReleaseID != rel.ID {
+				return errors.New("update attachement of release permission denied")
+			}
+		}
+
+		for uuid, newName := range editAttachments {
+			if !deletedUUIDsMap[uuid] {
+				if err = models.UpdateAttachmentByUUID(ctx, &models.Attachment{
+					UUID: uuid,
+					Name: newName,
+				}, "name"); err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	if err = commiter.Commit(); err != nil {
+		return
+	}
+
+	for _, uuid := range delAttachmentUUIDs {
+		if err := storage.Attachments.Delete(models.AttachmentRelativePath(uuid)); err != nil {
+			// Even delete files failed, but the attachments has been removed from database, so we
+			// should not return error but only record the error on logs.
+			// users have to delete this attachments manually or we should have a
+			// synchronize between database attachment table and attachment storage
+			log.Error("delete attachment[uuid: %s] failed: %v", uuid, err)
+		}
 	}
 
-	if !isCreate {
+	if !isCreated {
 		notification.NotifyUpdateRelease(doer, rel)
 		return
 	}