refactor(avatar): share avatar-upload logic between v1 and v2 handlers

Author: kolaente

pkg/modules/avatar/avatar.go

@@ -17,6 +17,10 @@
 package avatar
 
 import (
+	"errors"
+	"io"
+	"strings"
+
 	"code.vikunja.io/api/pkg/log"
 	"code.vikunja.io/api/pkg/modules/avatar/botmarble"
 	"code.vikunja.io/api/pkg/modules/avatar/empty"
@@ -27,8 +31,15 @@ import (
 	"code.vikunja.io/api/pkg/modules/avatar/openid"
 	"code.vikunja.io/api/pkg/modules/avatar/upload"
 	"code.vikunja.io/api/pkg/user"
+
+	"github.com/gabriel-vasile/mimetype"
+	"xorm.io/xorm"
 )
 
+// ErrNotAnImage is returned by StoreUploadedAvatar when the uploaded file is not an image.
+// Callers should map this to a 400 Bad Request response.
+var ErrNotAnImage = errors.New("uploaded file is no image")
+
 // Provider defines the avatar provider interface
 type Provider interface {
 	// GetAvatar is the method used to get an actual avatar for a user
@@ -82,3 +93,32 @@ func GetProvider(u *user.User) Provider {
 		return &empty.Provider{}
 	}
 }
+
+// StoreUploadedAvatar validates that src is an image, switches the user's avatar
+// provider to "upload", stores the image as the user's avatar and flushes all
+// cached avatars for the user. It returns ErrNotAnImage if src is not an image.
+//
+// This is the shared core used by both the v1 and v2 avatar upload handlers; the
+// callers are responsible for the transport-specific parts (reading the uploaded
+// file and writing the response).
+func StoreUploadedAvatar(s *xorm.Session, u *user.User, src io.ReadSeeker) error {
+	mime, err := mimetype.DetectReader(src)
+	if err != nil {
+		return err
+	}
+	if !strings.HasPrefix(mime.String(), "image") {
+		return ErrNotAnImage
+	}
+	if _, err := src.Seek(0, io.SeekStart); err != nil {
+		return err
+	}
+
+	u.AvatarProvider = "upload"
+	if err := upload.StoreAvatarFile(s, u, src); err != nil {
+		return err
+	}
+
+	FlushAllCaches(u)
+
+	return nil
+}

pkg/routes/api/v1/avatar.go

@@ -24,15 +24,12 @@ import (
 	"code.vikunja.io/api/pkg/modules/avatar"
 	"code.vikunja.io/api/pkg/modules/avatar/botmarble"
 	"code.vikunja.io/api/pkg/modules/avatar/empty"
-	"code.vikunja.io/api/pkg/modules/avatar/upload"
 	"code.vikunja.io/api/pkg/user"
 
-	"io"
+	"errors"
 	"net/http"
 	"strconv"
-	"strings"
 
-	"github.com/gabriel-vasile/mimetype"
 	"github.com/labstack/echo/v5"
 )
 
@@ -137,21 +134,11 @@ func UploadAvatar(c *echo.Context) (err error) {
 	}
 	defer src.Close()
 
-	// Validate we're dealing with an image
-	mime, err := mimetype.DetectReader(src)
-	if err != nil {
-		_ = s.Rollback()
-		return err
-	}
-	if !strings.HasPrefix(mime.String(), "image") {
-		return c.JSON(http.StatusBadRequest, models.Message{Message: "Uploaded file is no image."})
-	}
-	_, _ = src.Seek(0, io.SeekStart)
-
-	u.AvatarProvider = "upload"
-	err = upload.StoreAvatarFile(s, u, src)
-	if err != nil {
+	if err := avatar.StoreUploadedAvatar(s, u, src); err != nil {
 		_ = s.Rollback()
+		if errors.Is(err, avatar.ErrNotAnImage) {
+			return c.JSON(http.StatusBadRequest, models.Message{Message: "Uploaded file is no image."})
+		}
 		return err
 	}
 
@@ -160,7 +147,5 @@ func UploadAvatar(c *echo.Context) (err error) {
 		return err
 	}
 
-	avatar.FlushAllCaches(u)
-
 	return c.JSON(http.StatusOK, models.Message{Message: "Avatar was uploaded successfully."})
 }

pkg/routes/api/v2/avatar_upload.go

@@ -18,23 +18,20 @@ package apiv2
 
 import (
 	"context"
-	"io"
+	"errors"
 	"net/http"
-	"strings"
 
 	"code.vikunja.io/api/pkg/config"
 	"code.vikunja.io/api/pkg/db"
 	"code.vikunja.io/api/pkg/models"
 	"code.vikunja.io/api/pkg/modules/avatar"
-	"code.vikunja.io/api/pkg/modules/avatar/upload"
 	"code.vikunja.io/api/pkg/user"
 
 	"github.com/danielgtaylor/huma/v2"
-	"github.com/gabriel-vasile/mimetype"
 )
 
 type avatarUploadInput struct {
-	// Broad allow-list because Huma's MimeTypeValidator rejects the part pre-handler; octet-stream covers programmatic clients. The real gate is mimetype.DetectReader in the handler.
+	// Broad allow-list because Huma's MimeTypeValidator rejects the part pre-handler; octet-stream covers programmatic clients. The real gate is the byte-level image check in avatar.StoreUploadedAvatar.
 	RawBody huma.MultipartFormFiles[struct {
 		Avatar huma.FormFile `form:"avatar" contentType:"image/png,image/jpeg,image/gif,image/webp,image/svg+xml,application/octet-stream" required:"true" doc:"The avatar image to upload. Must be an image; it is resized server-side and re-encoded as PNG."`
 	}]
@@ -86,24 +83,11 @@ func avatarUpload(ctx context.Context, in *avatarUploadInput) (*avatarUploadBody
 	src := in.RawBody.Data().Avatar
 	defer func() { _ = src.Close() }()
 
-	// Byte-level image check, same allow-list as v1's UploadAvatar.
-	mime, err := mimetype.DetectReader(src)
-	if err != nil {
-		_ = s.Rollback()
-		return nil, translateDomainError(err)
-	}
-	if !strings.HasPrefix(mime.String(), "image") {
-		_ = s.Rollback()
-		return nil, huma.Error400BadRequest("Uploaded file is no image.")
-	}
-	if _, err := src.Seek(0, io.SeekStart); err != nil {
-		_ = s.Rollback()
-		return nil, translateDomainError(err)
-	}
-
-	u.AvatarProvider = "upload"
-	if err := upload.StoreAvatarFile(s, u, src); err != nil {
+	if err := avatar.StoreUploadedAvatar(s, u, src); err != nil {
 		_ = s.Rollback()
+		if errors.Is(err, avatar.ErrNotAnImage) {
+			return nil, huma.Error400BadRequest("Uploaded file is no image.")
+		}
 		return nil, translateDomainError(err)
 	}
 
@@ -112,7 +96,5 @@ func avatarUpload(ctx context.Context, in *avatarUploadInput) (*avatarUploadBody
 		return nil, translateDomainError(err)
 	}
 
-	avatar.FlushAllCaches(u)
-
 	return &avatarUploadBody{Body: &models.Message{Message: "Avatar was uploaded successfully."}}, nil
 }