Merge pull request 'Implement external release assets' (#1445) from maltejur/forgejo:forgejo-external-attachments into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/1445
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

routers/api/v1/repo/release.go

@@ -247,7 +247,7 @@ func CreateRelease(ctx *context.APIContext) {
 			IsTag:            false,
 			Repo:             ctx.Repo.Repository,
 		}
-		if err := release_service.CreateRelease(ctx.Repo.GitRepo, rel, nil, ""); err != nil {
+		if err := release_service.CreateRelease(ctx.Repo.GitRepo, rel, "", nil); err != nil {
 			if repo_model.IsErrReleaseAlreadyExist(err) {
 				ctx.Error(http.StatusConflict, "ReleaseAlreadyExist", err)
 			} else if models.IsErrProtectedTagName(err) {
@@ -274,7 +274,7 @@ func CreateRelease(ctx *context.APIContext) {
 		rel.Publisher = ctx.Doer
 		rel.Target = form.Target
 
-		if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil, true); err != nil {
+		if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, true, nil); err != nil {
 			ctx.Error(http.StatusInternalServerError, "UpdateRelease", err)
 			return
 		}
@@ -351,7 +351,7 @@ func EditRelease(ctx *context.APIContext) {
 	if form.HideArchiveLinks != nil {
 		rel.HideArchiveLinks = *form.HideArchiveLinks
 	}
-	if err := release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, nil, nil, nil, false); err != nil {
+	if err := release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, false, nil); err != nil {
 		ctx.Error(http.StatusInternalServerError, "UpdateRelease", err)
 		return
 	}

routers/api/v1/repo/release_attachment.go

@@ -5,7 +5,10 @@ package repo
 
 import (
 	"io"
+	"mime/multipart"
 	"net/http"
+	"net/url"
+	"path"
 	"strings"
 
 	repo_model "code.gitea.io/gitea/models/repo"
@@ -179,11 +182,18 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
 	//   description: name of the attachment
 	//   type: string
 	//   required: false
+	// # There is no good way to specify "either 'attachment' or 'external_url' is required" with OpenAPI
+	// # https://github.com/OAI/OpenAPI-Specification/issues/256
 	// - name: attachment
 	//   in: formData
-	//   description: attachment to upload
+	//   description: attachment to upload (this parameter is incompatible with `external_url`)
 	//   type: file
 	//   required: false
+	// - name: external_url
+	//   in: formData
+	//   description: url to external asset (this parameter is incompatible with `attachment`)
+	//   type: string
+	//   required: false
 	// responses:
 	//   "201":
 	//     "$ref": "#/responses/Attachment"
@@ -205,51 +215,96 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
 	}
 
 	// Get uploaded file from request
-	var content io.ReadCloser
-	var filename string
-	var size int64 = -1
+	var isForm, hasAttachmentFile, hasExternalURL bool
+	externalURL := ctx.FormString("external_url")
+	hasExternalURL = externalURL != ""
+	filename := ctx.FormString("name")
+	isForm = strings.HasPrefix(strings.ToLower(ctx.Req.Header.Get("Content-Type")), "multipart/form-data")
 
-	if strings.HasPrefix(strings.ToLower(ctx.Req.Header.Get("Content-Type")), "multipart/form-data") {
-		file, header, err := ctx.Req.FormFile("attachment")
-		if err != nil {
-			ctx.Error(http.StatusInternalServerError, "GetFile", err)
-			return
-		}
-		defer file.Close()
-
-		content = file
-		size = header.Size
-		filename = header.Filename
-		if name := ctx.FormString("name"); name != "" {
-			filename = name
-		}
+	if isForm {
+		_, _, err := ctx.Req.FormFile("attachment")
+		hasAttachmentFile = err == nil
 	} else {
-		content = ctx.Req.Body
-		filename = ctx.FormString("name")
+		hasAttachmentFile = ctx.Req.Body != nil
 	}
 
-	if filename == "" {
-		ctx.Error(http.StatusBadRequest, "CreateReleaseAttachment", "Could not determine name of attachment.")
-		return
-	}
+	if hasAttachmentFile && hasExternalURL {
+		ctx.Error(http.StatusBadRequest, "DuplicateAttachment", "'attachment' and 'external_url' are mutually exclusive")
+	} else if hasAttachmentFile {
+		var content io.ReadCloser
+		var size int64 = -1
 
-	// Create a new attachment and save the file
-	attach, err := attachment.UploadAttachment(ctx, content, setting.Repository.Release.AllowedTypes, size, &repo_model.Attachment{
-		Name:       filename,
-		UploaderID: ctx.Doer.ID,
-		RepoID:     ctx.Repo.Repository.ID,
-		ReleaseID:  releaseID,
-	})
-	if err != nil {
-		if upload.IsErrFileTypeForbidden(err) {
-			ctx.Error(http.StatusBadRequest, "DetectContentType", err)
+		if isForm {
+			var header *multipart.FileHeader
+			content, header, _ = ctx.Req.FormFile("attachment")
+			size = header.Size
+			defer content.Close()
+			if filename == "" {
+				filename = header.Filename
+			}
+		} else {
+			content = ctx.Req.Body
+			defer content.Close()
+		}
+
+		if filename == "" {
+			ctx.Error(http.StatusBadRequest, "MissingName", "Missing 'name' parameter")
 			return
 		}
-		ctx.Error(http.StatusInternalServerError, "NewAttachment", err)
-		return
-	}
 
-	ctx.JSON(http.StatusCreated, convert.ToAPIAttachment(ctx.Repo.Repository, attach))
+		// Create a new attachment and save the file
+		attach, err := attachment.UploadAttachment(ctx, content, setting.Repository.Release.AllowedTypes, size, &repo_model.Attachment{
+			Name:       filename,
+			UploaderID: ctx.Doer.ID,
+			RepoID:     ctx.Repo.Repository.ID,
+			ReleaseID:  releaseID,
+		})
+		if err != nil {
+			if upload.IsErrFileTypeForbidden(err) {
+				ctx.Error(http.StatusBadRequest, "DetectContentType", err)
+				return
+			}
+			ctx.Error(http.StatusInternalServerError, "NewAttachment", err)
+			return
+		}
+
+		ctx.JSON(http.StatusCreated, convert.ToAPIAttachment(ctx.Repo.Repository, attach))
+	} else if hasExternalURL {
+		url, err := url.Parse(externalURL)
+		if err != nil {
+			ctx.Error(http.StatusBadRequest, "InvalidExternalURL", err)
+			return
+		}
+
+		if filename == "" {
+			filename = path.Base(url.Path)
+
+			if filename == "." {
+				// Url path is empty
+				filename = url.Host
+			}
+		}
+
+		attach, err := attachment.NewExternalAttachment(ctx, &repo_model.Attachment{
+			Name:        filename,
+			UploaderID:  ctx.Doer.ID,
+			RepoID:      ctx.Repo.Repository.ID,
+			ReleaseID:   releaseID,
+			ExternalURL: url.String(),
+		})
+		if err != nil {
+			if repo_model.IsErrInvalidExternalURL(err) {
+				ctx.Error(http.StatusBadRequest, "NewExternalAttachment", err)
+			} else {
+				ctx.Error(http.StatusInternalServerError, "NewExternalAttachment", err)
+			}
+			return
+		}
+
+		ctx.JSON(http.StatusCreated, convert.ToAPIAttachment(ctx.Repo.Repository, attach))
+	} else {
+		ctx.Error(http.StatusBadRequest, "MissingAttachment", "One of 'attachment' or 'external_url' is required")
+	}
 }
 
 // EditReleaseAttachment updates the given attachment
@@ -322,8 +377,21 @@ func EditReleaseAttachment(ctx *context.APIContext) {
 		attach.Name = form.Name
 	}
 
+	if form.DownloadURL != "" {
+		if attach.ExternalURL == "" {
+			ctx.Error(http.StatusBadRequest, "EditAttachment", "existing attachment is not external")
+			return
+		}
+		attach.ExternalURL = form.DownloadURL
+	}
+
 	if err := repo_model.UpdateAttachment(ctx, attach); err != nil {
-		ctx.Error(http.StatusInternalServerError, "UpdateAttachment", attach)
+		if repo_model.IsErrInvalidExternalURL(err) {
+			ctx.Error(http.StatusBadRequest, "UpdateAttachment", err)
+		} else {
+			ctx.Error(http.StatusInternalServerError, "UpdateAttachment", err)
+		}
+		return
 	}
 	ctx.JSON(http.StatusCreated, convert.ToAPIAttachment(ctx.Repo.Repository, attach))
 }

routers/web/repo/attachment.go

@@ -122,6 +122,11 @@ func ServeAttachment(ctx *context.Context, uuid string) {
 		}
 	}
 
+	if attach.ExternalURL != "" {
+		ctx.Redirect(attach.ExternalURL)
+		return
+	}
+
 	if err := attach.IncreaseDownloadCount(ctx); err != nil {
 		ctx.ServerError("IncreaseDownloadCount", err)
 		return

routers/web/repo/release.go

@@ -18,6 +18,7 @@ import (
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/base"
+	"code.gitea.io/gitea/modules/container"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/gitrepo"
 	"code.gitea.io/gitea/modules/log"
@@ -491,9 +492,44 @@ func NewReleasePost(ctx *context.Context) {
 		return
 	}
 
-	var attachmentUUIDs []string
+	attachmentChanges := make(container.Set[*releaseservice.AttachmentChange])
+	attachmentChangesByID := make(map[string]*releaseservice.AttachmentChange)
+
 	if setting.Attachment.Enabled {
-		attachmentUUIDs = form.Files
+		for _, uuid := range form.Files {
+			attachmentChanges.Add(&releaseservice.AttachmentChange{
+				Action: "add",
+				Type:   "attachment",
+				UUID:   uuid,
+			})
+		}
+
+		const namePrefix = "attachment-new-name-"
+		const exturlPrefix = "attachment-new-exturl-"
+		for k, v := range ctx.Req.Form {
+			isNewName := strings.HasPrefix(k, namePrefix)
+			isNewExturl := strings.HasPrefix(k, exturlPrefix)
+			if isNewName || isNewExturl {
+				var id string
+				if isNewName {
+					id = k[len(namePrefix):]
+				} else if isNewExturl {
+					id = k[len(exturlPrefix):]
+				}
+				if _, ok := attachmentChangesByID[id]; !ok {
+					attachmentChangesByID[id] = &releaseservice.AttachmentChange{
+						Action: "add",
+						Type:   "external",
+					}
+					attachmentChanges.Add(attachmentChangesByID[id])
+				}
+				if isNewName {
+					attachmentChangesByID[id].Name = v[0]
+				} else if isNewExturl {
+					attachmentChangesByID[id].ExternalURL = v[0]
+				}
+			}
+		}
 	}
 
 	rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, form.TagName)
@@ -553,7 +589,7 @@ func NewReleasePost(ctx *context.Context) {
 			IsTag:            false,
 		}
 
-		if err = releaseservice.CreateRelease(ctx.Repo.GitRepo, rel, attachmentUUIDs, msg); err != nil {
+		if err = releaseservice.CreateRelease(ctx.Repo.GitRepo, rel, msg, attachmentChanges.Values()); err != nil {
 			ctx.Data["Err_TagName"] = true
 			switch {
 			case repo_model.IsErrReleaseAlreadyExist(err):
@@ -562,6 +598,8 @@ func NewReleasePost(ctx *context.Context) {
 				ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_invalid"), tplReleaseNew, &form)
 			case models.IsErrProtectedTagName(err):
 				ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_protected"), tplReleaseNew, &form)
+			case repo_model.IsErrInvalidExternalURL(err):
+				ctx.RenderWithErr(ctx.Tr("repo.release.invalid_external_url", err.(repo_model.ErrInvalidExternalURL).ExternalURL), tplReleaseNew, &form)
 			default:
 				ctx.ServerError("CreateRelease", err)
 			}
@@ -583,9 +621,14 @@ func NewReleasePost(ctx *context.Context) {
 		rel.HideArchiveLinks = form.HideArchiveLinks
 		rel.IsTag = false
 
-		if err = releaseservice.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, attachmentUUIDs, nil, nil, true); err != nil {
+		if err = releaseservice.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, true, attachmentChanges.Values()); err != nil {
 			ctx.Data["Err_TagName"] = true
-			ctx.ServerError("UpdateRelease", err)
+			switch {
+			case repo_model.IsErrInvalidExternalURL(err):
+				ctx.RenderWithErr(ctx.Tr("repo.release.invalid_external_url", err.(repo_model.ErrInvalidExternalURL).ExternalURL), tplReleaseNew, &form)
+			default:
+				ctx.ServerError("UpdateRelease", err)
+			}
 			return
 		}
 	}
@@ -667,6 +710,15 @@ func EditReleasePost(ctx *context.Context) {
 	ctx.Data["prerelease"] = rel.IsPrerelease
 	ctx.Data["hide_archive_links"] = rel.HideArchiveLinks
 
+	rel.Repo = ctx.Repo.Repository
+	if err := rel.LoadAttributes(ctx); err != nil {
+		ctx.ServerError("LoadAttributes", err)
+		return
+	}
+	// TODO: If an error occurs, do not forget the attachment edits the user made
+	// when displaying the error message.
+	ctx.Data["attachments"] = rel.Attachments
+
 	if ctx.HasError() {
 		ctx.HTML(http.StatusOK, tplReleaseNew)
 		return
@@ -674,15 +726,67 @@ func EditReleasePost(ctx *context.Context) {
 
 	const delPrefix = "attachment-del-"
 	const editPrefix = "attachment-edit-"
-	var addAttachmentUUIDs, delAttachmentUUIDs []string
-	editAttachments := make(map[string]string) // uuid -> new name
+	const newPrefix = "attachment-new-"
+	const namePrefix = "name-"
+	const exturlPrefix = "exturl-"
+	attachmentChanges := make(container.Set[*releaseservice.AttachmentChange])
+	attachmentChangesByID := make(map[string]*releaseservice.AttachmentChange)
+
 	if setting.Attachment.Enabled {
-		addAttachmentUUIDs = form.Files
+		for _, uuid := range form.Files {
+			attachmentChanges.Add(&releaseservice.AttachmentChange{
+				Action: "add",
+				Type:   "attachment",
+				UUID:   uuid,
+			})
+		}
+
 		for k, v := range ctx.Req.Form {
 			if strings.HasPrefix(k, delPrefix) && v[0] == "true" {
-				delAttachmentUUIDs = append(delAttachmentUUIDs, k[len(delPrefix):])
-			} else if strings.HasPrefix(k, editPrefix) {
-				editAttachments[k[len(editPrefix):]] = v[0]
+				attachmentChanges.Add(&releaseservice.AttachmentChange{
+					Action: "delete",
+					UUID:   k[len(delPrefix):],
+				})
+			} else {
+				isUpdatedName := strings.HasPrefix(k, editPrefix+namePrefix)
+				isUpdatedExturl := strings.HasPrefix(k, editPrefix+exturlPrefix)
+				isNewName := strings.HasPrefix(k, newPrefix+namePrefix)
+				isNewExturl := strings.HasPrefix(k, newPrefix+exturlPrefix)
+
+				if isUpdatedName || isUpdatedExturl || isNewName || isNewExturl {
+					var uuid string
+
+					if isUpdatedName {
+						uuid = k[len(editPrefix+namePrefix):]
+					} else if isUpdatedExturl {
+						uuid = k[len(editPrefix+exturlPrefix):]
+					} else if isNewName {
+						uuid = k[len(newPrefix+namePrefix):]
+					} else if isNewExturl {
+						uuid = k[len(newPrefix+exturlPrefix):]
+					}
+
+					if _, ok := attachmentChangesByID[uuid]; !ok {
+						attachmentChangesByID[uuid] = &releaseservice.AttachmentChange{
+							Type: "attachment",
+							UUID: uuid,
+						}
+						attachmentChanges.Add(attachmentChangesByID[uuid])
+					}
+
+					if isUpdatedName || isUpdatedExturl {
+						attachmentChangesByID[uuid].Action = "update"
+					} else if isNewName || isNewExturl {
+						attachmentChangesByID[uuid].Action = "add"
+					}
+
+					if isUpdatedName || isNewName {
+						attachmentChangesByID[uuid].Name = v[0]
+					} else if isUpdatedExturl || isNewExturl {
+						attachmentChangesByID[uuid].ExternalURL = v[0]
+						attachmentChangesByID[uuid].Type = "external"
+					}
+				}
 			}
 		}
 	}
@@ -692,9 +796,13 @@ func EditReleasePost(ctx *context.Context) {
 	rel.IsDraft = len(form.Draft) > 0
 	rel.IsPrerelease = form.Prerelease
 	rel.HideArchiveLinks = form.HideArchiveLinks
-	if err = releaseservice.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo,
-		rel, addAttachmentUUIDs, delAttachmentUUIDs, editAttachments, false); err != nil {
-		ctx.ServerError("UpdateRelease", err)
+	if err = releaseservice.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, false, attachmentChanges.Values()); err != nil {
+		switch {
+		case repo_model.IsErrInvalidExternalURL(err):
+			ctx.RenderWithErr(ctx.Tr("repo.release.invalid_external_url", err.(repo_model.ErrInvalidExternalURL).ExternalURL), tplReleaseNew, &form)
+		default:
+			ctx.ServerError("UpdateRelease", err)
+		}
 		return
 	}
 	ctx.Redirect(ctx.Repo.RepoLink + "/releases")