kodev-org
/
vikunja
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(task-attachment): share upload+download via pkg/web/files for v1+v2

This commit is contained in:
kolaente 2026-06-10 10:27:34 +02:00 committed by kolaente
parent 5cdc785b49
commit cec74717fc
5 changed files with 246 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.gitignore vendored
View File

@ -26,6 +26,7 @@ docs/resources/
pkg/static/templates_vfsdata.go
files/
!pkg/files/
!pkg/web/files/
vikunja-dump*
vendor/
os-packages/

69
pkg/models/task_attachment.go
View File

@ -23,6 +23,7 @@ import (
"image/png"
"io"
"strconv"
"strings"
"time"
"code.vikunja.io/api/pkg/events"
@ -106,6 +107,74 @@ func (ta *TaskAttachment) NewAttachment(s *xorm.Session, f io.ReadSeeker, realna
return nil
}
// AttachmentToUpload is a transport-neutral file to attach, so the upload logic
// can be shared by the multipart v1 handler and the Huma v2 handler.
type AttachmentToUpload struct {
Reader io.ReadSeeker
Filename string
Size uint64
}
// UploadTaskAttachments checks create access to the task, then stores each file,
// collecting per-file failures rather than aborting. The caller owns the session
// and the commit. A returned err means the request as a whole failed (e.g.
// forbidden); per-file failures come back in failures instead.
func UploadTaskAttachments(s *xorm.Session, a web.Auth, taskID int64, uploads []*AttachmentToUpload) (success []*TaskAttachment, failures []error, err error) {
ta := &TaskAttachment{TaskID: taskID}
can, err := ta.CanCreate(s, a)
if err != nil {
return nil, nil, err
}
if !can {
return nil, nil, ErrGenericForbidden{}
}
for _, upload := range uploads {
attachment := &TaskAttachment{TaskID: taskID}
if err := attachment.NewAttachment(s, upload.Reader, upload.Filename, upload.Size, a); err != nil {
failures = append(failures, err)
continue
}
success = append(success, attachment)
}
return success, failures, nil
}
// LoadTaskAttachmentForDownload checks read access, loads the attachment with its
// open file, and resolves a preview if previewSize is set and the file is an image.
// It returns the loaded attachment and, when applicable, the preview bytes (the
// caller serves those instead of the file). The caller owns the session, the
// commit, and writing the response. Returns ErrGenericForbidden on denied access.
func LoadTaskAttachmentForDownload(s *xorm.Session, a web.Auth, taskID, attachmentID int64, previewSize PreviewSize) (ta *TaskAttachment, preview []byte, err error) {
ta = &TaskAttachment{ID: attachmentID, TaskID: taskID}
can, _, err := ta.CanRead(s, a)
if err != nil {
return nil, nil, err
}
if !can {
return nil, nil, ErrGenericForbidden{}
}
if err := ta.ReadOne(s, a); err != nil {
return nil, nil, err
}
if err := ta.File.LoadFileByID(); err != nil {
return nil, nil, err
}
if previewSize != PreviewSizeUnknown && strings.HasPrefix(ta.File.Mime, "image") {
preview = ta.GetPreview(previewSize)
// GetPreview consumes the file reader; re-open it for the non-preview fallback.
if preview == nil {
if err := ta.File.LoadFileByID(); err != nil {
return nil, nil, err
}
}
}
return ta, preview, nil
}
// ReadOne returns a task attachment
func (ta *TaskAttachment) ReadOne(s *xorm.Session, _ web.Auth) (err error) {
query := s.Where("id = ?", ta.ID).NoAutoCondition()

133
pkg/routes/api/v1/task_attachment.go
View File

@ -18,43 +18,16 @@ package v1
import (
"errors"
"io"
"mime"
"net/http"
"strconv"
"strings"
"code.vikunja.io/api/pkg/config"
"code.vikunja.io/api/pkg/db"
"code.vikunja.io/api/pkg/models"
auth2 "code.vikunja.io/api/pkg/modules/auth"
"code.vikunja.io/api/pkg/web"
webfiles "code.vikunja.io/api/pkg/web/files"
"github.com/labstack/echo/v5"
)
// attachmentUploadError represents a structured error for attachment upload failures
type attachmentUploadError struct {
Code int `json:"code,omitempty"`
Message string `json:"message"`
}
// toAttachmentUploadError converts an error to a structured attachmentUploadError
func toAttachmentUploadError(err error) attachmentUploadError {
// Try to get structured error info from HTTPErrorProcessor
if httpErr, ok := err.(web.HTTPErrorProcessor); ok {
errDetails := httpErr.HTTPError()
return attachmentUploadError{
Code: errDetails.Code,
Message: errDetails.Message,
}
}
// Fall back to just the error message
return attachmentUploadError{
Message: err.Error(),
}
}
// UploadTaskAttachment handles everything needed for the upload of a task attachment
// @Summary Upload a task attachment
// @Description Upload a task attachment. You can pass multiple files with the files form param.
@ -76,7 +49,6 @@ func UploadTaskAttachment(c *echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "No task ID provided").Wrap(err)
}
// Permissions check
auth, err := auth2.GetAuthFromClaims(c)
if err != nil {
return err
@ -85,15 +57,6 @@ func UploadTaskAttachment(c *echo.Context) error {
s := db.NewSession()
defer s.Close()
can, err := taskAttachment.CanCreate(s, auth)
if err != nil {
_ = s.Rollback()
return err
}
if !can {
return echo.ErrForbidden
}
// Multipart form
form, err := c.MultipartForm()
if err != nil {
@ -104,31 +67,23 @@ func UploadTaskAttachment(c *echo.Context) error {
return err
}
type result struct {
Errors []attachmentUploadError `json:"errors"`
Success []*models.TaskAttachment `json:"success"`
}
r := &result{}
fileHeaders := form.File["files"]
uploads := make([]*models.AttachmentToUpload, 0, len(fileHeaders))
var openErrors []error
for _, file := range fileHeaders {
// We create a new attachment object here to have a clean start
ta := &models.TaskAttachment{
TaskID: taskAttachment.TaskID,
}
f, err := file.Open()
if err != nil {
r.Errors = append(r.Errors, toAttachmentUploadError(err))
openErrors = append(openErrors, err)
continue
}
defer f.Close()
uploads = append(uploads, &models.AttachmentToUpload{Reader: f, Filename: file.Filename, Size: uint64(file.Size)})
}
err = ta.NewAttachment(s, f, file.Filename, uint64(file.Size), auth)
if err != nil {
r.Errors = append(r.Errors, toAttachmentUploadError(err))
continue
}
r.Success = append(r.Success, ta)
success, failures, err := models.UploadTaskAttachments(s, auth, taskAttachment.TaskID, uploads)
if err != nil {
_ = s.Rollback()
return err
}
if err := s.Commit(); err != nil {
@ -136,7 +91,7 @@ func UploadTaskAttachment(c *echo.Context) error {
return err
}
return c.JSON(http.StatusOK, r)
return c.JSON(http.StatusOK, webfiles.BuildUploadResult(success, append(openErrors, failures...)))
}
// GetTaskAttachment returns a task attachment to download for the user
@ -160,7 +115,6 @@ func GetTaskAttachment(c *echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "No task ID provided").Wrap(err)
}
// Permissions check
auth, err := auth2.GetAuthFromClaims(c)
if err != nil {
return err
@ -169,36 +123,11 @@ func GetTaskAttachment(c *echo.Context) error {
s := db.NewSession()
defer s.Close()
can, _, err := taskAttachment.CanRead(s, auth)
if err != nil {
_ = s.Rollback()
return err
}
if !can {
return echo.ErrForbidden
}
// Get the attachment incl file
err = taskAttachment.ReadOne(s, auth)
if err != nil {
_ = s.Rollback()
return err
}
// Open the file so its content is available for preview generation and download
err = taskAttachment.File.LoadFileByID()
if err != nil {
_ = s.Rollback()
return err
}
// If the preview query parameter is set, get the preview (cached or generate)
previewSize := models.GetPreviewSizeFromString(c.QueryParam("preview_size"))
if previewSize != models.PreviewSizeUnknown && strings.HasPrefix(taskAttachment.File.Mime, "image") {
previewFileBytes := taskAttachment.GetPreview(previewSize)
if previewFileBytes != nil {
return c.Blob(http.StatusOK, "image/png", previewFileBytes)
}
attachment, preview, err := models.LoadTaskAttachmentForDownload(s, auth, taskAttachment.TaskID, taskAttachment.ID, previewSize)
if err != nil {
_ = s.Rollback()
return err
}
if err := s.Commit(); err != nil {
@ -206,36 +135,6 @@ func GetTaskAttachment(c *echo.Context) error {
return err
}
mimeToReturn := taskAttachment.File.Mime
if mimeToReturn == "" {
mimeToReturn = "application/octet-stream"
}
c.Response().Header().Set("Content-Disposition", mime.FormatMediaType("attachment", map[string]string{
"filename": taskAttachment.File.Name,
}))
c.Response().Header().Set("Content-Type", mimeToReturn)
c.Response().Header().Set("Content-Length", strconv.FormatUint(taskAttachment.File.Size, 10))
c.Response().Header().Set("Last-Modified", taskAttachment.File.Created.UTC().Format(http.TimeFormat))
// Override the global no-store directive so browsers can cache attachments.
// no-cache allows caching but requires revalidation via If-Modified-Since.
c.Response().Header().Set("Cache-Control", "no-cache")
if config.FilesType.GetString() == "s3" {
// Check If-Modified-Since and return 304 if the file hasn't changed.
// http.ServeContent handles this automatically for local files.
if ifModSince := c.Request().Header.Get("If-Modified-Since"); ifModSince != "" {
if t, parseErr := http.ParseTime(ifModSince); parseErr == nil && !taskAttachment.File.Created.UTC().After(t) {
return c.NoContent(http.StatusNotModified)
}
}
// s3 files cannot use http.ServeContent as it requires a Seekable file
// so we stream the file content directly to the response
_, err = io.Copy(c.Response(), taskAttachment.File.File)
return err
}
http.ServeContent(c.Response(), c.Request(), taskAttachment.File.Name, taskAttachment.File.Created, taskAttachment.File.File.(io.ReadSeeker))
webfiles.WriteAttachmentDownload(c.Response(), c.Request(), attachment, preview)
return nil
}

104
pkg/web/files/task_attachment.go Normal file
View File

@ -0,0 +1,104 @@
// Vikunja is a to-do list application to facilitate your life.
// Copyright 2018-present Vikunja and contributors. All rights reserved.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
// Package files holds the HTTP-layer glue for serving task attachments —
// the upload-result DTOs and the download response writer — shared by the
// v1 and v2 handlers. The domain logic stays in pkg/models; this package
// only translates it to and from the wire.
package files
import (
"io"
"mime"
"net/http"
"strconv"
"code.vikunja.io/api/pkg/models"
"code.vikunja.io/api/pkg/web"
)
// AttachmentUploadError is a per-file upload failure.
type AttachmentUploadError struct {
Code int `json:"code,omitempty" doc:"Vikunja numeric error code, when the failure carries one."`
Message string `json:"message" doc:"A human-readable description of why this file failed."`
}
// AttachmentUploadResult is the outcome of an attachment upload: files are
// processed independently, so a per-file failure lands in Errors while the
// rest still succeed.
type AttachmentUploadResult struct {
Errors []AttachmentUploadError `json:"errors" doc:"Per-file failures. A file that fails here does not fail the whole request; the others still upload."`
Success []*models.TaskAttachment `json:"success" doc:"The attachments that were created successfully."`
}
// BuildUploadResult turns the domain function's plain return values into the
// wire DTO, mapping each failure to its numeric code when it carries one.
func BuildUploadResult(success []*models.TaskAttachment, failures []error) *AttachmentUploadResult {
r := &AttachmentUploadResult{Success: success}
for _, err := range failures {
r.Errors = append(r.Errors, toAttachmentUploadError(err))
}
return r
}
func toAttachmentUploadError(err error) AttachmentUploadError {
if httpErr, ok := err.(web.HTTPErrorProcessor); ok {
details := httpErr.HTTPError()
return AttachmentUploadError{Code: details.Code, Message: details.Message}
}
return AttachmentUploadError{Message: err.Error()}
}
// WriteAttachmentDownload streams the attachment (or its preview) to the response:
// http.ServeContent for seekable local files (Range + If-Modified-Since for free),
// a manual 304 + io.Copy otherwise. It closes the file reader.
func WriteAttachmentDownload(w http.ResponseWriter, r *http.Request, ta *models.TaskAttachment, preview []byte) {
defer func() { _ = ta.File.File.Close() }()
if preview != nil {
w.Header().Set("Content-Type", "image/png")
w.Header().Set("Content-Length", strconv.Itoa(len(preview)))
_, _ = w.Write(preview)
return
}
mimeToReturn := ta.File.Mime
if mimeToReturn == "" {
mimeToReturn = "application/octet-stream"
}
w.Header().Set("Content-Disposition", mime.FormatMediaType("attachment", map[string]string{"filename": ta.File.Name}))
w.Header().Set("Content-Type", mimeToReturn)
w.Header().Set("Content-Length", strconv.FormatUint(ta.File.Size, 10))
w.Header().Set("Last-Modified", ta.File.Created.UTC().Format(http.TimeFormat))
// Override the global no-store directive so browsers can cache attachments.
w.Header().Set("Cache-Control", "no-cache")
// Local files are *os.File (seekable), so ServeContent gives Range +
// If-Modified-Since for free; s3 (and the in-memory test storage) return a
// non-seekable reader, so check If-Modified-Since manually and io.Copy.
if seeker, ok := ta.File.File.(io.ReadSeeker); ok {
http.ServeContent(w, r, ta.File.Name, ta.File.Created, seeker)
return
}
if ifModSince := r.Header.Get("If-Modified-Since"); ifModSince != "" {
if t, parseErr := http.ParseTime(ifModSince); parseErr == nil && !ta.File.Created.UTC().After(t) {
w.WriteHeader(http.StatusNotModified)
return
}
}
_, _ = io.Copy(w, ta.File.File)
}

56
pkg/web/files/task_attachment_test.go Normal file
View File

@ -0,0 +1,56 @@
// Vikunja is a to-do list application to facilitate your life.
// Copyright 2018-present Vikunja and contributors. All rights reserved.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
package files
import (
"errors"
"testing"
"code.vikunja.io/api/pkg/models"
"github.com/stretchr/testify/assert"
)
func TestBuildUploadResult(t *testing.T) {
t.Run("maps a domain error to its numeric code", func(t *testing.T) {
// ErrTaskAttachmentIsTooLarge is an HTTPErrorProcessor, so its Code must surface.
r := BuildUploadResult(nil, []error{models.ErrTaskAttachmentIsTooLarge{Size: 99}})
assert.Empty(t, r.Success)
if assert.Len(t, r.Errors, 1) {
assert.Equal(t, models.ErrCodeTaskAttachmentIsTooLarge, r.Errors[0].Code)
assert.NotEmpty(t, r.Errors[0].Message)
}
})
t.Run("plain error has no code, just the message", func(t *testing.T) {
r := BuildUploadResult(nil, []error{errors.New("boom")})
if assert.Len(t, r.Errors, 1) {
assert.Zero(t, r.Errors[0].Code)
assert.Equal(t, "boom", r.Errors[0].Message)
}
})
t.Run("preserves success and failure order", func(t *testing.T) {
success := []*models.TaskAttachment{{ID: 1}, {ID: 2}}
r := BuildUploadResult(success, []error{errors.New("first"), errors.New("second")})
assert.Equal(t, success, r.Success)
if assert.Len(t, r.Errors, 2) {
assert.Equal(t, "first", r.Errors[0].Message)
assert.Equal(t, "second", r.Errors[1].Message)
}
})
}