refactor(avatar): share avatar resolution between v1 and v2 handlers

Extract the duplicated user-lookup, provider-selection and size-clamping
logic from the v1 GetAvatar and v2 avatarGet handlers into a single
avatar.GetAvatarForUsername helper. Both handlers now call it and keep
only their transport-specific code (v1: echo size parse + c.Blob, v2:
huma input/response). Pure refactor, behavior is unchanged.
This commit is contained in:
kolaente 2026-06-01 14:02:34 +02:00 committed by kolaente
parent a4a0af91ff
commit e81ccb3486
3 changed files with 47 additions and 61 deletions

View File

@ -17,6 +17,7 @@
package avatar
import (
"code.vikunja.io/api/pkg/config"
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/api/pkg/modules/avatar/botmarble"
"code.vikunja.io/api/pkg/modules/avatar/empty"
@ -27,6 +28,8 @@ import (
"code.vikunja.io/api/pkg/modules/avatar/openid"
"code.vikunja.io/api/pkg/modules/avatar/upload"
"code.vikunja.io/api/pkg/user"
"xorm.io/xorm"
)
// Provider defines the avatar provider interface
@ -58,6 +61,43 @@ func FlushAllCaches(u *user.User) {
}
}
// GetAvatarForUsername resolves and renders the avatar for a username. It is the
// shared core behind both the v1 and v2 avatar endpoints: it looks up the user,
// tolerates an unknown/disabled user (returning the default placeholder rather
// than an error, since avatars are loaded via <img> tags), picks the right
// provider (empty for unknown users, botmarble for bots, otherwise the user's
// configured provider) and clamps the size to the server's configured maximum.
func GetAvatarForUsername(s *xorm.Session, username string, size int64) (data []byte, mime string, err error) {
u, err := user.GetUserWithEmail(s, &user.User{Username: username})
if err != nil && !user.IsErrUserDoesNotExist(err) && !user.IsErrUserStatusError(err) {
log.Errorf("Error getting user for avatar: %v", err)
return nil, "", err
}
found := err == nil || user.IsErrUserStatusError(err)
provider := GetProvider(u)
if !found {
// Unknown user: serve the default placeholder.
provider = &empty.Provider{}
}
if found && u.IsBot() {
provider = &botmarble.Provider{}
}
if size > config.ServiceMaxAvatarSize.GetInt64() {
size = config.ServiceMaxAvatarSize.GetInt64()
}
data, mime, err = provider.GetAvatar(u, size)
if err != nil {
log.Errorf("Error getting avatar for user %d: %v", u.ID, err)
return nil, "", err
}
return data, mime, nil
}
// GetProvider returns the appropriate avatar provider for a user
func GetProvider(u *user.User) Provider {
provider := u.AvatarProvider

View File

@ -17,13 +17,10 @@
package v1
import (
"code.vikunja.io/api/pkg/config"
"code.vikunja.io/api/pkg/db"
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/api/pkg/models"
"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"
@ -54,42 +51,19 @@ func GetAvatar(c *echo.Context) error {
s := db.NewSession()
defer s.Close()
// Get the user
u, err := user.GetUserWithEmail(s, &user.User{Username: username})
if err != nil && !user.IsErrUserDoesNotExist(err) && !user.IsErrUserStatusError(err) {
log.Errorf("Error getting user for avatar: %v", err)
return err
}
found := err == nil || user.IsErrUserStatusError(err)
avatarProvider := avatar.GetProvider(u)
if !found {
avatarProvider = &empty.Provider{}
}
if found && u.IsBot() {
avatarProvider = &botmarble.Provider{}
}
size := c.QueryParam("size")
var sizeInt int64 = 250 // Default size of 250
if size != "" {
var err error
sizeInt, err = strconv.ParseInt(size, 10, 64)
if err != nil {
log.Errorf("Error parsing size: %v", err)
return models.ErrInvalidModel{Message: "Invalid size parameter"}
}
}
if sizeInt > config.ServiceMaxAvatarSize.GetInt64() {
sizeInt = config.ServiceMaxAvatarSize.GetInt64()
}
// Get the avatar
a, mimeType, err := avatarProvider.GetAvatar(u, sizeInt)
a, mimeType, err := avatar.GetAvatarForUsername(s, username, sizeInt)
if err != nil {
log.Errorf("Error getting avatar for user %d: %v", u.ID, err)
return err
}

View File

@ -20,13 +20,8 @@ import (
"context"
"net/http"
"code.vikunja.io/api/pkg/config"
"code.vikunja.io/api/pkg/db"
"code.vikunja.io/api/pkg/log"
"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/user"
"github.com/danielgtaylor/huma/v2"
)
@ -72,10 +67,8 @@ func RegisterAvatarRoutes(api huma.API) {
}
func avatarGet(ctx context.Context, in *avatarInput) (*avatarResponse, error) {
// Pull auth so the endpoint behaves as authenticated even though it reads
// no per-user permission (any authenticated caller may view any avatar,
// matching v1). The token middleware already rejects anonymous requests;
// this surfaces a clean 401 if a handler is somehow reached without auth.
// Authenticated but no per-user check — any authenticated caller may view any
// avatar (matching v1); authFromCtx just surfaces a clean 401 if auth is missing.
if _, err := authFromCtx(ctx); err != nil {
return nil, err
}
@ -83,31 +76,10 @@ func avatarGet(ctx context.Context, in *avatarInput) (*avatarResponse, error) {
s := db.NewSession()
defer s.Close()
u, err := user.GetUserWithEmail(s, &user.User{Username: in.Username})
if err != nil && !user.IsErrUserDoesNotExist(err) && !user.IsErrUserStatusError(err) {
log.Errorf("Error getting user for avatar: %v", err)
return nil, translateDomainError(err)
}
found := err == nil || user.IsErrUserStatusError(err)
avatarProvider := avatar.GetProvider(u)
if !found {
// Unknown user: serve the default placeholder, exactly like v1.
avatarProvider = &empty.Provider{}
}
if found && u.IsBot() {
avatarProvider = &botmarble.Provider{}
}
size := in.Size
if size > config.ServiceMaxAvatarSize.GetInt64() {
size = config.ServiceMaxAvatarSize.GetInt64()
}
a, mimeType, err := avatarProvider.GetAvatar(u, size)
// Avatar resolution (user lookup, provider selection, size clamping) is
// shared with the v1 handler; only the transport differs.
a, mimeType, err := avatar.GetAvatarForUsername(s, in.Username, in.Size)
if err != nil {
log.Errorf("Error getting avatar for user %d: %v", u.ID, err)
return nil, translateDomainError(err)
}