diff --git a/cmd/doctor.go b/cmd/doctor.go index c2ddfef9c5..c5e6bc7916 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -20,6 +20,7 @@ import ( migrate_base "forgejo.org/models/gitea_migrations/base" repo_model "forgejo.org/models/repo" user_model "forgejo.org/models/user" + "forgejo.org/modules/avatarstore" "forgejo.org/modules/container" "forgejo.org/modules/log" "forgejo.org/modules/setting" @@ -28,6 +29,7 @@ import ( exif_terminator "code.superseriousbusiness.org/exif-terminator" "github.com/urfave/cli/v3" + "xorm.io/builder" ) // CmdDoctor represents the available doctor sub-command. @@ -43,6 +45,7 @@ func cmdDoctor() *cli.Command { cmdDoctorConvert(), cmdAvatarStripExif(), cmdCleanupCommitStatuses(), + cmdResizeAvatars(), }, } } @@ -117,6 +120,30 @@ func cmdAvatarStripExif() *cli.Command { } } +func cmdResizeAvatars() *cli.Command { + return &cli.Command{ + Name: "avatar-resize", + Usage: "Generate resized versions of user or repository avatars", + Description: `Forgejo serves small versions of avatars for inclusion in the web UI. + +Those rescaled versions are computed on-demand and cached in the avatar storage. + +This command pre-computes rescaled versions of avatars ahead of time.`, + Before: noDanglingArgs, + Action: runAvatarResize, + Flags: []cli.Flag{ + &cli.BoolFlag{ + Name: "user", + Usage: "Resize the user avatars", + }, + &cli.BoolFlag{ + Name: "repository", + Usage: "Resize the repository avatars", + }, + }, + } +} + func cmdCleanupCommitStatuses() *cli.Command { return &cli.Command{ Name: "cleanup-commit-status", @@ -373,6 +400,79 @@ func runAvatarStripExif(ctx context.Context, c *cli.Command) error { return nil } +func precomputeResizedAvatars(imgStorage storage.ObjectStorage, imgPath string, maxOriginSize int64) error { + // Load the avatar + avatarBytes, err := imgStorage.Open(imgPath) + if err != nil { + return err + } + meta, err := avatarBytes.Stat() + if err != nil { + return err + } + // If the avatar is small enough, don't compute resized versions for it. + // This makes it possible to preserve animated avatars when they are small enough. + if meta.Size() < maxOriginSize { + return nil + } + img, _, err := image.Decode(avatarBytes) + if err != nil { + return err + } + return avatarstore.PrecomputeResizedAvatars(imgStorage, img, imgPath) +} + +func runAvatarResize(ctx context.Context, c *cli.Command) error { + ctx, cancel := installSignals(ctx) + defer cancel() + + if err := initDB(ctx); err != nil { + return err + } + + if err := storage.Init(); err != nil { + return err + } + + runUser := c.Bool("user") + runRepo := c.Bool("repository") + return RunAvatarResize(ctx, runUser, runRepo) +} + +func RunAvatarResize(ctx context.Context, runUser, runRepo bool) error { + if !runUser && !runRepo { + return fmt.Errorf("at least one of --user or --repository should be provided") + } + + if runUser { + log.Info("Resizing user avatars") + if err := db.Iterate( + ctx, + builder.Neq{"avatar": ""}, + func(ctx context.Context, user *user_model.User) error { + return precomputeResizedAvatars(storage.Avatars, user.Avatar, setting.Avatar.MaxOriginSize) + }, + ); err != nil { + return err + } + } + + if runRepo { + log.Info("Resizing repository avatars") + if err := db.Iterate( + ctx, + builder.Neq{"avatar": ""}, + func(ctx context.Context, repo *repo_model.Repository) error { + return precomputeResizedAvatars(storage.RepoAvatars, repo.Avatar, setting.Avatar.MaxOriginSize) + }, + ); err != nil { + return err + } + } + + return nil +} + func runCleanupCommitStatus(ctx context.Context, cli *cli.Command) error { ctx, cancel := installSignals(ctx) defer cancel() diff --git a/models/avatars/avatar.go b/models/avatars/avatar.go index 43d52c562b..7e884983f5 100644 --- a/models/avatars/avatar.go +++ b/models/avatars/avatar.go @@ -15,6 +15,7 @@ import ( "sync/atomic" "forgejo.org/models/db" + "forgejo.org/modules/avatar" "forgejo.org/modules/cache" "forgejo.org/modules/log" "forgejo.org/modules/setting" @@ -169,6 +170,15 @@ func GenerateUserAvatarImageLink(userAvatar string) string { return setting.AppSubURL + "/avatars/" + url.PathEscape(userAvatar) } +// GenerateUserResizedAvatarLink returns the URL to the smallest resized version of the avatar bigger than the supplied size +func GenerateUserResizedAvatarLink(userAvatar string, requestedSize int) string { + cachedSize := avatar.BestAvatarCachedSize(requestedSize) + if cachedSize == 0 { + return GenerateUserAvatarImageLink(userAvatar) + } + return fmt.Sprintf("%s/avatars/%s?size=%d", setting.AppSubURL, url.PathEscape(userAvatar), cachedSize) +} + // generateRecognizedAvatarURL generate a recognized avatar (Gravatar/Libravatar) URL, it modifies the URL so the parameter is passed by a copy func generateRecognizedAvatarURL(u url.URL, size int) string { urlQuery := u.Query() diff --git a/models/avatars/avatar_test.go b/models/avatars/avatar_test.go index 7850d2c096..d43433e446 100644 --- a/models/avatars/avatar_test.go +++ b/models/avatars/avatar_test.go @@ -9,6 +9,7 @@ import ( avatars_model "forgejo.org/models/avatars" "forgejo.org/models/db" system_model "forgejo.org/models/system" + "forgejo.org/modules/avatar" "forgejo.org/modules/setting" "forgejo.org/modules/setting/config" @@ -57,3 +58,10 @@ func TestSizedAvatarLink(t *testing.T) { avatars_model.GenerateEmailAvatarFastLink(db.DefaultContext, "gitea@example.com", 100), ) } + +func TestBestAvatarCachedSize(t *testing.T) { + assert.Equal(t, 64, avatar.BestAvatarCachedSize(2)) + assert.Equal(t, 64, avatar.BestAvatarCachedSize(64)) + assert.Equal(t, 128, avatar.BestAvatarCachedSize(65)) + assert.Equal(t, 0, avatar.BestAvatarCachedSize(1000)) +} diff --git a/models/repo/avatar.go b/models/repo/avatar.go index a108fda62d..cafcd679ef 100644 --- a/models/repo/avatar.go +++ b/models/repo/avatar.go @@ -32,7 +32,7 @@ func ExistsWithAvatarAtStoragePath(ctx context.Context, storagePath string) (boo // RelAvatarLink returns a relative link to the repository's avatar. func (repo *Repository) RelAvatarLink(ctx context.Context) string { - return repo.relAvatarLink(ctx) + return repo.relAvatarLink(ctx, 0) } // generateRandomAvatar generates a random avatar for repository. @@ -65,7 +65,7 @@ func generateRandomAvatar(ctx context.Context, repo *Repository) error { return nil } -func (repo *Repository) relAvatarLink(ctx context.Context) string { +func (repo *Repository) relAvatarLink(ctx context.Context, size int) string { // If no avatar - path is empty avatarPath := repo.CustomAvatarRelativePath() if len(avatarPath) == 0 { @@ -81,12 +81,21 @@ func (repo *Repository) relAvatarLink(ctx context.Context) string { return "" } } - return setting.AppSubURL + "/repo-avatars/" + url.PathEscape(repo.Avatar) + cachedSize := avatar.BestAvatarCachedSize(size) + if cachedSize == 0 { + return setting.AppSubURL + "/repo-avatars/" + url.PathEscape(repo.Avatar) + } + return fmt.Sprintf("%s/repo-avatars/%s?size=%d", setting.AppSubURL, url.PathEscape(repo.Avatar), cachedSize) } // AvatarLink returns a link to the repository's avatar. func (repo *Repository) AvatarLink(ctx context.Context) string { - link := repo.relAvatarLink(ctx) + return repo.AvatarLinkWithSize(ctx, 0) +} + +// Returns URL to the smallest resized version of the avatar bigger than the supplied size +func (repo *Repository) AvatarLinkWithSize(ctx context.Context, size int) string { + link := repo.relAvatarLink(ctx, size) // we only prepend our AppURL to our known (relative, internal) avatar link to get an absolute URL if strings.HasPrefix(link, "/") && !strings.HasPrefix(link, "//") { return setting.AppURL + strings.TrimPrefix(link, setting.AppSubURL)[1:] diff --git a/models/user/avatar.go b/models/user/avatar.go index 726d67f5e0..5180e4ef2b 100644 --- a/models/user/avatar.go +++ b/models/user/avatar.go @@ -60,8 +60,8 @@ func GenerateRandomAvatar(ctx context.Context, u *User) error { return nil } -// AvatarLinkWithSize returns a link to the user's avatar. Size is only used for -// GenerateEmailAvatarFastLink, for external email-based avatar services +// AvatarLinkWithSize returns a link to the user's avatar. It may be a relative +// or absolute URL. func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string { if u.IsGhost() || u.ID <= 0 { return avatars.DefaultAvatarLink() @@ -89,7 +89,7 @@ func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string { if u.Avatar == "" { return avatars.DefaultAvatarLink() } - return avatars.GenerateUserAvatarImageLink(u.Avatar) + return avatars.GenerateUserResizedAvatarLink(u.Avatar, size) } return avatars.GenerateEmailAvatarFastLink(ctx, u.AvatarEmail, size) } diff --git a/models/user/avatar_test.go b/models/user/avatar_test.go index d3a164142d..6c458b8a8e 100644 --- a/models/user/avatar_test.go +++ b/models/user/avatar_test.go @@ -32,6 +32,14 @@ func TestUserAvatarLink(t *testing.T) { assert.Equal(t, "https://localhost/sub-path/avatars/avatar.png", link) } +func TestUserAvatarLinkWithSize(t *testing.T) { + u := &User{ID: 1, Avatar: "avatar.png"} + link := u.AvatarLinkWithSize(db.DefaultContext, 12) + assert.Equal(t, "/avatars/avatar.png?size=64", link) + link = u.AvatarLinkWithSize(db.DefaultContext, 2048) + assert.Equal(t, "/avatars/avatar.png", link) +} + func TestUserAvatarGenerate(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) var err error diff --git a/modules/avatar/avatar.go b/modules/avatar/avatar.go index 5ab872f953..781c664a9b 100644 --- a/modules/avatar/avatar.go +++ b/modules/avatar/avatar.go @@ -30,6 +30,9 @@ import ( // than the size after resizing. const DefaultAvatarSize = 256 +// Sizes to which we allow resizing an avatar down to. They must be specified in increasing order. +var AllowedResizedAvatarSizes = []int{64, 128} + // RandomImageSize generates and returns a random avatar image unique to input data // in custom size (height and width). func RandomImageSize(size int, data []byte) (image.Image, error) { @@ -49,34 +52,34 @@ func RandomImage(data []byte) (image.Image, error) { // processAvatarImage process the avatar image data, crop and resize it if necessary. // the returned data could be the original image if no processing is needed. -func processAvatarImage(data []byte, maxOriginSize int64) ([]byte, error) { +func processAvatarImage(data []byte, maxOriginSize int64) ([]byte, image.Image, error) { imgCfg, imgType, err := image.DecodeConfig(bytes.NewReader(data)) if err != nil { - return nil, fmt.Errorf("image.DecodeConfig: %w", err) + return nil, nil, fmt.Errorf("image.DecodeConfig: %w", err) } // for safety, only accept known types explicitly if imgType != "png" && imgType != "jpeg" && imgType != "gif" && imgType != "webp" { - return nil, errors.New("unsupported avatar image type") + return nil, nil, errors.New("unsupported avatar image type") } // do not process image which is too large, it would consume too much memory if imgCfg.Width > setting.Avatar.MaxWidth { - return nil, fmt.Errorf("image width is too large: %d > %d", imgCfg.Width, setting.Avatar.MaxWidth) + return nil, nil, fmt.Errorf("image width is too large: %d > %d", imgCfg.Width, setting.Avatar.MaxWidth) } if imgCfg.Height > setting.Avatar.MaxHeight { - return nil, fmt.Errorf("image height is too large: %d > %d", imgCfg.Height, setting.Avatar.MaxHeight) + return nil, nil, fmt.Errorf("image height is too large: %d > %d", imgCfg.Height, setting.Avatar.MaxHeight) } var cleanedBytes []byte if imgType != "gif" { // "gif" is the only imgType supported above, but not supported by exif_terminator cleanedData, err := exif_terminator.Terminate(bytes.NewReader(data), imgType) if err != nil { - return nil, fmt.Errorf("error cleaning exif data: %w", err) + return nil, nil, fmt.Errorf("error cleaning exif data: %w", err) } cleanedBytes, err = io.ReadAll(cleanedData) if err != nil { - return nil, fmt.Errorf("error reading cleaned data: %w", err) + return nil, nil, fmt.Errorf("error reading cleaned data: %w", err) } } else { // gif cleanedBytes = data @@ -87,43 +90,44 @@ func processAvatarImage(data []byte, maxOriginSize int64) ([]byte, error) { // And one more thing, webp is not fully supported, for animated webp, image.DecodeConfig works but Decode fails. // So for animated webp, if the uploaded file is smaller than maxOriginSize, it will be used, if it's larger, there will be an error. if len(data) < int(maxOriginSize) { - return cleanedBytes, nil + //nolint:nilnil + return cleanedBytes, nil, nil } img, _, err := image.Decode(bytes.NewReader(cleanedBytes)) if err != nil { - return nil, fmt.Errorf("image.Decode: %w", err) + return nil, nil, fmt.Errorf("image.Decode: %w", err) } // try to crop and resize the origin image if necessary img = cropSquare(img) targetSize := DefaultAvatarSize * setting.Avatar.RenderedSizeFactor - img = scale(img, targetSize, targetSize, draw.BiLinear) + img = Scale(img, targetSize, targetSize, draw.BiLinear) // try to encode the cropped/resized image to png bs := bytes.Buffer{} if err = png.Encode(&bs, img); err != nil { - return nil, err + return nil, nil, err } resized := bs.Bytes() // usually the png compression is not good enough, use the original image (no cropping/resizing) if the origin is smaller if len(data) <= len(resized) { - return cleanedBytes, nil + return cleanedBytes, img, nil } - return resized, nil + return resized, img, nil } // ProcessAvatarImage process the avatar image data, crop and resize it if necessary. // the returned data could be the original image if no processing is needed. -func ProcessAvatarImage(data []byte) ([]byte, error) { +func ProcessAvatarImage(data []byte) ([]byte, image.Image, error) { return processAvatarImage(data, setting.Avatar.MaxOriginSize) } -// scale resizes the image to width x height using the given scaler. -func scale(src image.Image, width, height int, scale draw.Scaler) image.Image { +// Scale resizes the image to width x height using the given scaler. +func Scale(src image.Image, width, height int, scale draw.Scaler) image.Image { rect := image.Rect(0, 0, width, height) dst := image.NewRGBA(rect) scale.Scale(dst, rect, src, src.Bounds(), draw.Over, nil) @@ -153,3 +157,17 @@ func cropSquare(src image.Image) image.Image { draw.Draw(dst, rect, src, rect.Min, draw.Src) return dst } + +// BestAvatarCachedSize computes the size at which the avatar should be downloaded, to display it at the desired size. +// When it returns 0, it means that the original should be downloaded. +func BestAvatarCachedSize(size int) int { + if size == 0 { + return 0 + } + for i := 0; i < len(AllowedResizedAvatarSizes); i++ { + if AllowedResizedAvatarSizes[i] >= size { + return AllowedResizedAvatarSizes[i] + } + } + return 0 +} diff --git a/modules/avatar/avatar_test.go b/modules/avatar/avatar_test.go index 7a395c49cc..b77f0e592b 100644 --- a/modules/avatar/avatar_test.go +++ b/modules/avatar/avatar_test.go @@ -39,8 +39,9 @@ func Test_ProcessAvatarPNG(t *testing.T) { data, err := os.ReadFile("testdata/avatar.png") require.NoError(t, err) - _, err = processAvatarImage(data, 262144) + _, img, err := processAvatarImage(data, 262144) require.NoError(t, err) + require.Nil(t, img) } func Test_ProcessAvatarJPEG(t *testing.T) { @@ -50,8 +51,9 @@ func Test_ProcessAvatarJPEG(t *testing.T) { data, err := os.ReadFile("testdata/avatar.jpeg") require.NoError(t, err) - _, err = processAvatarImage(data, 262144) + _, img, err := processAvatarImage(data, 262144) require.NoError(t, err) + require.Nil(t, img) } func Test_ProcessAvatarGIF(t *testing.T) { @@ -61,15 +63,16 @@ func Test_ProcessAvatarGIF(t *testing.T) { data, err := os.ReadFile("testdata/avatar.gif") require.NoError(t, err) - _, err = processAvatarImage(data, 262144) + _, img, err := processAvatarImage(data, 262144) require.NoError(t, err) + require.Nil(t, img) } func Test_ProcessAvatarInvalidData(t *testing.T) { defer test.MockVariableValue(&setting.Avatar.MaxWidth, 5)() defer test.MockVariableValue(&setting.Avatar.MaxHeight, 5)() - _, err := processAvatarImage([]byte{}, 12800) + _, _, err := processAvatarImage([]byte{}, 12800) assert.EqualError(t, err, "image.DecodeConfig: image: unknown format") } @@ -80,7 +83,7 @@ func Test_ProcessAvatarInvalidImageSize(t *testing.T) { data, err := os.ReadFile("testdata/avatar.png") require.NoError(t, err) - _, err = processAvatarImage(data, 12800) + _, _, err = processAvatarImage(data, 12800) assert.EqualError(t, err, "image width is too large: 10 > 5") } @@ -104,49 +107,53 @@ func Test_ProcessAvatarImage(t *testing.T) { // if origin image canvas is too large, crop and resize it origin := newImgData(500, 600) - result, err := processAvatarImage(origin, 0) + result, img, err := processAvatarImage(origin, 0) require.NoError(t, err) assert.NotEqual(t, origin, result) decoded, err := png.Decode(bytes.NewReader(result)) require.NoError(t, err) assert.Equal(t, scaledSize, decoded.Bounds().Max.X) assert.Equal(t, scaledSize, decoded.Bounds().Max.Y) + assert.Equal(t, scaledSize, img.Bounds().Max.X) + assert.Equal(t, scaledSize, img.Bounds().Max.Y) // if origin image is smaller than the default size, use the origin image origin = newImgData(1) - result, err = processAvatarImage(origin, 0) + result, _, err = processAvatarImage(origin, 0) require.NoError(t, err) assert.Equal(t, origin, result) // use the origin image if the origin is smaller origin = newImgData(scaledSize + 100) - result, err = processAvatarImage(origin, 0) + result, _, err = processAvatarImage(origin, 0) require.NoError(t, err) assert.Less(t, len(result), len(origin)) // still use the origin image if the origin doesn't exceed the max-origin-size origin = newImgData(scaledSize + 100) - result, err = processAvatarImage(origin, 262144) + result, img, err = processAvatarImage(origin, 262144) require.NoError(t, err) + require.Nil(t, img) assert.Equal(t, origin, result) // allow to use known image format (eg: webp) if it is small enough origin, err = os.ReadFile("testdata/animated.webp") require.NoError(t, err) - result, err = processAvatarImage(origin, 262144) + result, img, err = processAvatarImage(origin, 262144) require.NoError(t, err) + require.Nil(t, img) assert.Equal(t, origin, result) // do not support unknown image formats, eg: SVG may contain embedded JS origin = []byte("") - _, err = processAvatarImage(origin, 262144) + _, _, err = processAvatarImage(origin, 262144) require.ErrorContains(t, err, "image: unknown format") // make sure the canvas size limit works setting.Avatar.MaxWidth = 5 setting.Avatar.MaxHeight = 5 origin = newImgData(10) - _, err = processAvatarImage(origin, 262144) + _, _, err = processAvatarImage(origin, 262144) require.ErrorContains(t, err, "image width is too large: 10 > 5") } @@ -173,7 +180,7 @@ func Test_ProcessAvatarExif(t *testing.T) { data, err := os.ReadFile("testdata/exif.jpg") require.NoError(t, err) - processedData, err := processAvatarImage(data, 12800) + processedData, _, err := processAvatarImage(data, 12800) require.NoError(t, err) safeExifJpeg(t, processedData) }) @@ -181,7 +188,7 @@ func Test_ProcessAvatarExif(t *testing.T) { data, err := os.ReadFile("testdata/exif.jpg") require.NoError(t, err) - processedData, err := processAvatarImage(data, 128000) + processedData, _, err := processAvatarImage(data, 128000) require.NoError(t, err) safeExifJpeg(t, processedData) }) diff --git a/modules/avatarstore/avatarstore.go b/modules/avatarstore/avatarstore.go new file mode 100644 index 0000000000..22f7b9a53a --- /dev/null +++ b/modules/avatarstore/avatarstore.go @@ -0,0 +1,66 @@ +// Copyright 2026 the Forgejo authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package avatarstore + +import ( + "errors" + "fmt" + "image" + "image/png" + "io" + "os" + + "forgejo.org/modules/avatar" + "forgejo.org/modules/log" + "forgejo.org/modules/storage" + + "golang.org/x/image/draw" +) + +// StoreAvatar stores an avatar in an object store and precomputes resized versions of it +func StoreAvatar(avatarPath string, avatarData []byte, avatarImg image.Image, imgStore storage.ObjectStorage) error { + err := storage.SaveFrom(imgStore, avatarPath, func(w io.Writer) error { + _, err := w.Write(avatarData) + return err + }) + if err != nil { + return err + } + if avatarImg != nil { + // pre-compute rescaled versions of the avatar + return PrecomputeResizedAvatars(imgStore, avatarImg, avatarPath) + } + return nil +} + +// PrecomputeResizedAvatars computes resized versions of the avatars and stores them in the cache +func PrecomputeResizedAvatars(resizedStore storage.ObjectStorage, avatarImg image.Image, avatarPath string) error { + for _, size := range avatar.AllowedResizedAvatarSizes { + rescaled := avatar.Scale(avatarImg, size, size, draw.BiLinear) + err := storage.SaveFrom(resizedStore, fmt.Sprintf("resized/%d/%s", size, avatarPath), func(w io.Writer) error { + return png.Encode(w, rescaled) + }) + if err != nil { + return err + } + } + return nil +} + +// DeleteAvatar removes an avatar file from both the original and resized storage +func DeleteAvatar(avatarPath string, imgStore storage.ObjectStorage) error { + if err := imgStore.Delete(avatarPath); err != nil { + if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("failed to remove %s: %w", avatarPath, err) + } + log.Warn("Deleting avatar %s but it doesn't exist", avatarPath) + } + for _, size := range avatar.AllowedResizedAvatarSizes { + err := imgStore.Delete(fmt.Sprintf("resized/%d/%s", size, avatarPath)) + if err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("failed to remove resized avatar resized/%d/%s: %w", size, avatarPath, err) + } + } + return nil +} diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index ef998f7916..7fe47c82aa 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -124,7 +124,7 @@ func TestPushCommits_AvatarLink(t *testing.T) { } assert.Equal(t, - "/avatars/ab53a2911ddf9b4817ac01ddcd3d975f", + "/avatars/ab53a2911ddf9b4817ac01ddcd3d975f?size=64", pushCommits.AvatarLink(db.DefaultContext, "user2@example.com")) assert.Equal(t, diff --git a/routers/web/base.go b/routers/web/base.go deleted file mode 100644 index c1bc7fef5e..0000000000 --- a/routers/web/base.go +++ /dev/null @@ -1,98 +0,0 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package web - -import ( - "errors" - "fmt" - "net/http" - "os" - "path" - "strings" - - "forgejo.org/modules/httpcache" - "forgejo.org/modules/log" - "forgejo.org/modules/setting" - "forgejo.org/modules/storage" - "forgejo.org/modules/util" - "forgejo.org/modules/web/routing" -) - -func storageHandler(storageSetting *setting.Storage, prefix string, objStore storage.ObjectStorage) http.HandlerFunc { - prefix = strings.Trim(prefix, "/") - funcInfo := routing.GetFuncInfo(storageHandler, prefix) - - if storageSetting.MinioConfig.ServeDirect { - return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - if req.Method != "GET" && req.Method != "HEAD" { - http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) - return - } - - if !strings.HasPrefix(req.URL.Path, "/"+prefix+"/") { - http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) - return - } - routing.UpdateFuncInfo(req.Context(), funcInfo) - - rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/") - rPath = util.PathJoinRelX(rPath) - - u, err := objStore.URL(rPath, path.Base(rPath), nil) - if err != nil { - if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) { - log.Warn("Unable to find %s %s", prefix, rPath) - http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) - return - } - log.Error("Error whilst getting URL for %s %s. Error: %v", prefix, rPath, err) - http.Error(w, fmt.Sprintf("Error whilst getting URL for %s %s", prefix, rPath), http.StatusInternalServerError) - return - } - - http.Redirect(w, req, u.String(), http.StatusTemporaryRedirect) - }) - } - - return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - if req.Method != "GET" && req.Method != "HEAD" { - http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) - return - } - - if !strings.HasPrefix(req.URL.Path, "/"+prefix+"/") { - http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) - return - } - routing.UpdateFuncInfo(req.Context(), funcInfo) - - rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/") - rPath = util.PathJoinRelX(rPath) - if rPath == "" || rPath == "." { - http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) - return - } - - fi, err := objStore.Stat(rPath) - if err != nil { - if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) { - log.Warn("Unable to find %s %s", prefix, rPath) - http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) - return - } - log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err) - http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath), http.StatusInternalServerError) - return - } - - fr, err := objStore.Open(rPath) - if err != nil { - log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err) - http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath), http.StatusInternalServerError) - return - } - defer fr.Close() - httpcache.ServeContentWithCacheControl(w, req, path.Base(rPath), fi.ModTime(), fr) - }) -} diff --git a/routers/web/resizing.go b/routers/web/resizing.go new file mode 100644 index 0000000000..b0daad3e96 --- /dev/null +++ b/routers/web/resizing.go @@ -0,0 +1,184 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package web + +import ( + "bytes" + "errors" + "fmt" + "image" + "image/png" + "io" + "net/http" + "os" + "path" + "slices" + "strconv" + "strings" + "time" + + "forgejo.org/modules/avatar" + "forgejo.org/modules/httpcache" + "forgejo.org/modules/log" + "forgejo.org/modules/setting" + "forgejo.org/modules/storage" + "forgejo.org/modules/util" + "forgejo.org/modules/web/routing" + + "golang.org/x/image/draw" +) + +// resizingHandler resizes images to one of the supported sizes. +// It expects URLs of the form "{prefix}/{size}/{image_id}" +func resizingHandler(prefix string, imgStore storage.ObjectStorage, allowedSizes []int) http.HandlerFunc { + whenMissing := func(path string, size int) ([]byte, error) { + return resizeImageFromStorage(imgStore, path, size, allowedSizes) + } + return cachingHandler(prefix, imgStore, whenMissing) +} + +// resizeImageFromStorage retrieves an image from an ObjectStorage and resizes it +func resizeImageFromStorage(imgStore storage.ObjectStorage, imgPath string, targetSize int, allowedSizes []int) ([]byte, error) { + if !slices.Contains(allowedSizes, targetSize) { + return nil, errors.New("invalid image size requested") + } + + // Get the original image + reader, err := imgStore.Open(imgPath) + if err != nil { + return nil, err + } + // Read the bytes in memory for the purpose of re-using them if the image is small + originalBytes, err := io.ReadAll(reader) + if err != nil { + return nil, err + } + originalReader := bytes.NewReader(originalBytes) + // Decode it as an image + image, _, err := image.Decode(originalReader) + if err != nil { + return nil, err + } + + buffer := new(bytes.Buffer) + + width := image.Bounds().Dx() + height := image.Bounds().Dy() + + if width <= targetSize && height <= targetSize { + // The original image is smaller than the requested size, + // just return it directly. + // This will still put a copy of it in the storage for the + // requested size which we could avoid, but that will also + // avoid the need for decoding the image next time it is requested. + return originalBytes, err + } + + thumbnail := avatar.Scale(image, targetSize, targetSize, draw.BiLinear) + err = png.Encode(buffer, thumbnail) + return buffer.Bytes(), err +} + +// cachingHandler serves blobs from an ObjectStorage, +// computing them on the fly if they are missing. It falls back on the original image if no size parameter is supplied. +func cachingHandler(prefix string, imgStore storage.ObjectStorage, whenMissing func(string, int) ([]byte, error)) http.HandlerFunc { + prefix = strings.Trim(prefix, "/") + funcInfo := routing.GetFuncInfo(cachingHandler, prefix) + + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if req.Method != "GET" && req.Method != "HEAD" { + http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) + return + } + + if !strings.HasPrefix(req.URL.Path, "/"+prefix+"/") { + http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) + return + } + routing.UpdateFuncInfo(req.Context(), funcInfo) + + sizeParam := req.URL.Query().Get("size") + if sizeParam == "" { + sizeParam = "0" + } + size, err := strconv.Atoi(sizeParam) + if err != nil { + http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) + return + } + + rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/") + rPath = util.PathJoinRelX(rPath) + if rPath == "" || rPath == "." || strings.Contains(rPath, "/") { + http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) + return + } + + if size != 0 && !slices.Contains(avatar.AllowedResizedAvatarSizes, size) { + http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) + return + } + + originalFile, err := imgStore.Stat(rPath) + if err != nil { + http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) + return + } + + // If no size is provided, or if the original file is small enough, fall back on the original image. + // This is primarily for the purpose of preserving animated images that are small enough, because + // the image-resizing code would remove the animation from the images otherwise. + // An alternative could be to only preserve such images if they are actually animated, but it would + // require reading the image and and inspecting its contents. + if size == 0 || originalFile.Size() < setting.Avatar.MaxOriginSize { + reader, err := imgStore.Open(rPath) + if err != nil { + log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err) + http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath), http.StatusInternalServerError) + return + } + defer reader.Close() + httpcache.ServeContentWithCacheControl(w, req, path.Base(rPath), originalFile.ModTime(), reader) + return + } + + cachePath := fmt.Sprintf("resized/%d/%s", size, rPath) + + fi, err := imgStore.Stat(cachePath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + // attempt to compute the missing value via the callback provided + computed, err := whenMissing(rPath, size) + if err != nil { + log.Warn("Unable to compute %s %s", prefix, rPath) + http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound) + return + } + reader := bytes.NewReader(computed) + _, err = imgStore.Save(cachePath, reader, int64(len(computed))) + if err != nil { + // only log the error, still return the computed resource without caching it + log.Warn("Unable to save %s %s: %s", prefix, cachePath, err) + } + + reader = bytes.NewReader(computed) + httpcache.ServeContentWithCacheControl(w, req, path.Base(cachePath), time.Now(), reader) + return + } + log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err) + http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath), http.StatusInternalServerError) + return + } + + fr, err := imgStore.Open(cachePath) + if err != nil { + log.Error("Error whilst opening %s %s. Error: %v", prefix, cachePath, err) + http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, cachePath), http.StatusInternalServerError) + return + } + defer fr.Close() + httpcache.ServeContentWithCacheControl(w, req, path.Base(rPath), fi.ModTime(), fr) + }) +} diff --git a/routers/web/web.go b/routers/web/web.go index 0e0559f557..97d7ddae0f 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -15,6 +15,7 @@ import ( "forgejo.org/models/perm" quota_model "forgejo.org/models/quota" "forgejo.org/models/unit" + "forgejo.org/modules/avatar" "forgejo.org/modules/log" "forgejo.org/modules/metrics" "forgejo.org/modules/public" @@ -276,8 +277,8 @@ func Routes() *web.Route { routes.Head("/", misc.DummyOK) // for health check - doesn't need to be passed through gzip handler routes.Methods("GET, HEAD, OPTIONS", "/assets/*", optionsCorsHandler(), public.FileHandlerFunc()) - routes.Methods("GET, HEAD", "/avatars/*", storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars)) - routes.Methods("GET, HEAD", "/repo-avatars/*", storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars)) + routes.Methods("GET, HEAD", "/avatars/*", resizingHandler("avatars", storage.Avatars, avatar.AllowedResizedAvatarSizes)) + routes.Methods("GET, HEAD", "/repo-avatars/*", resizingHandler("repo-avatars", storage.RepoAvatars, avatar.AllowedResizedAvatarSizes)) routes.Methods("GET, HEAD", "/apple-touch-icon.png", misc.StaticRedirect("/assets/img/apple-touch-icon.png")) routes.Methods("GET, HEAD", "/apple-touch-icon-precomposed.png", misc.StaticRedirect("/assets/img/apple-touch-icon.png")) routes.Methods("GET, HEAD", "/favicon.ico", misc.StaticRedirect("/assets/img/favicon.png")) diff --git a/services/doctor/lfs.go b/services/doctor/lfs.go index 35a02b6cf0..650787c54c 100644 --- a/services/doctor/lfs.go +++ b/services/doctor/lfs.go @@ -48,5 +48,5 @@ func garbageCollectLFSCheck(ctx context.Context, logger log.Logger, autofix bool return err } - return checkStorage(&checkStorageOptions{LFS: true})(ctx, logger, autofix) + return CheckStorage(&CheckStorageOptions{LFS: true})(ctx, logger, autofix) } diff --git a/services/doctor/storage.go b/services/doctor/storage.go index 7dbe475d6c..f008228928 100644 --- a/services/doctor/storage.go +++ b/services/doctor/storage.go @@ -79,7 +79,7 @@ func commonCheckStorage(logger log.Logger, autofix bool, opts *commonStorageChec return nil } -type checkStorageOptions struct { +type CheckStorageOptions struct { All bool Attachments bool LFS bool @@ -89,8 +89,8 @@ type checkStorageOptions struct { Packages bool } -// checkStorage will return a doctor check function to check the requested storage types for "orphaned" stored object/files and optionally delete them -func checkStorage(opts *checkStorageOptions) func(ctx context.Context, logger log.Logger, autofix bool) error { +// CheckStorage will return a doctor check function to check the requested storage types for "orphaned" stored object/files and optionally delete them +func CheckStorage(opts *CheckStorageOptions) func(ctx context.Context, logger log.Logger, autofix bool) error { return func(ctx context.Context, logger log.Logger, autofix bool) error { if err := storage.Init(); err != nil { logger.Error("storage.Init failed: %v", err) @@ -136,7 +136,13 @@ func checkStorage(opts *checkStorageOptions) func(ctx context.Context, logger lo &commonStorageCheckOptions{ storer: storage.Avatars, isOrphaned: func(path string, obj storage.Object, stat fs.FileInfo) (bool, error) { - exists, err := user.ExistsWithAvatarAtStoragePath(ctx, path) + // The path is either just a hash, if the file is an original avatar uploaded by the user, + // or of the form "resized//" if it is a resized version of the avatar. + // In both cases, we retain the file if and only if the hash corresponds to an avatar + // of an existing user. + pathParts := strings.Split(path, "/") + hash := pathParts[len(pathParts)-1] + exists, err := user.ExistsWithAvatarAtStoragePath(ctx, hash) return !exists, err }, name: "avatar", @@ -150,7 +156,10 @@ func checkStorage(opts *checkStorageOptions) func(ctx context.Context, logger lo &commonStorageCheckOptions{ storer: storage.RepoAvatars, isOrphaned: func(path string, obj storage.Object, stat fs.FileInfo) (bool, error) { - exists, err := repo.ExistsWithAvatarAtStoragePath(ctx, path) + // See the comment above to explain the handling of original and resized avatars. + pathParts := strings.Split(path, "/") + hash := pathParts[len(pathParts)-1] + exists, err := repo.ExistsWithAvatarAtStoragePath(ctx, hash) return !exists, err }, name: "repo avatar", @@ -212,7 +221,7 @@ func init() { Title: "Check if there are orphaned storage files", Name: "storages", IsDefault: false, - Run: checkStorage(&checkStorageOptions{All: true}), + Run: CheckStorage(&CheckStorageOptions{All: true}), AbortIfFailed: false, SkipDatabaseInitialization: false, Priority: 1, @@ -222,7 +231,7 @@ func init() { Title: "Check if there are orphaned attachments in storage", Name: "storage-attachments", IsDefault: false, - Run: checkStorage(&checkStorageOptions{Attachments: true}), + Run: CheckStorage(&CheckStorageOptions{Attachments: true}), AbortIfFailed: false, SkipDatabaseInitialization: false, Priority: 1, @@ -232,7 +241,7 @@ func init() { Title: "Check if there are orphaned lfs files in storage", Name: "storage-lfs", IsDefault: false, - Run: checkStorage(&checkStorageOptions{LFS: true}), + Run: CheckStorage(&CheckStorageOptions{LFS: true}), AbortIfFailed: false, SkipDatabaseInitialization: false, Priority: 1, @@ -242,7 +251,7 @@ func init() { Title: "Check if there are orphaned avatars in storage", Name: "storage-avatars", IsDefault: false, - Run: checkStorage(&checkStorageOptions{Avatars: true, RepoAvatars: true}), + Run: CheckStorage(&CheckStorageOptions{Avatars: true, RepoAvatars: true}), AbortIfFailed: false, SkipDatabaseInitialization: false, Priority: 1, @@ -252,7 +261,7 @@ func init() { Title: "Check if there are orphaned archives in storage", Name: "storage-archives", IsDefault: false, - Run: checkStorage(&checkStorageOptions{RepoArchives: true}), + Run: CheckStorage(&CheckStorageOptions{RepoArchives: true}), AbortIfFailed: false, SkipDatabaseInitialization: false, Priority: 1, @@ -262,7 +271,7 @@ func init() { Title: "Check if there are orphaned package blobs in storage", Name: "storage-packages", IsDefault: false, - Run: checkStorage(&checkStorageOptions{Packages: true}), + Run: CheckStorage(&CheckStorageOptions{Packages: true}), AbortIfFailed: false, SkipDatabaseInitialization: false, Priority: 1, diff --git a/services/repository/avatar.go b/services/repository/avatar.go index a1cd3228df..ab1944e2e5 100644 --- a/services/repository/avatar.go +++ b/services/repository/avatar.go @@ -12,6 +12,7 @@ import ( "forgejo.org/models/db" repo_model "forgejo.org/models/repo" "forgejo.org/modules/avatar" + "forgejo.org/modules/avatarstore" "forgejo.org/modules/log" "forgejo.org/modules/storage" ) @@ -19,7 +20,7 @@ import ( // UploadAvatar saves custom avatar for repository. // FIXME: split uploads to different subdirs in case we have massive number of repos. func UploadAvatar(ctx context.Context, repo *repo_model.Repository, data []byte) error { - avatarData, err := avatar.ProcessAvatarImage(data) + avatarData, img, err := avatar.ProcessAvatarImage(data) if err != nil { return err } @@ -44,15 +45,12 @@ func UploadAvatar(ctx context.Context, repo *repo_model.Repository, data []byte) return fmt.Errorf("UploadAvatar: Update repository avatar: %w", err) } - if err := storage.SaveFrom(storage.RepoAvatars, repo.CustomAvatarRelativePath(), func(w io.Writer) error { - _, err := w.Write(avatarData) - return err - }); err != nil { - return fmt.Errorf("UploadAvatar %s failed: Failed to remove old repo avatar %s: %w", repo.RepoPath(), newAvatar, err) + if err := avatarstore.StoreAvatar(repo.CustomAvatarRelativePath(), avatarData, img, storage.RepoAvatars); err != nil { + return fmt.Errorf("UploadAvatar %s failed: Failed to store repo avatar %s: %w", repo.RepoPath(), newAvatar, err) } if len(oldAvatarPath) > 0 { - if err := storage.RepoAvatars.Delete(oldAvatarPath); err != nil { + if err := avatarstore.DeleteAvatar(oldAvatarPath, storage.RepoAvatars); err != nil { return fmt.Errorf("UploadAvatar: Failed to remove old repo avatar %s: %w", oldAvatarPath, err) } } @@ -81,7 +79,7 @@ func DeleteAvatar(ctx context.Context, repo *repo_model.Repository) error { return fmt.Errorf("DeleteAvatar: Update repository avatar: %w", err) } - if err := storage.RepoAvatars.Delete(avatarPath); err != nil { + if err := avatarstore.DeleteAvatar(avatarPath, storage.RepoAvatars); err != nil { return fmt.Errorf("DeleteAvatar: Failed to remove %s: %w", avatarPath, err) } diff --git a/services/user/avatar.go b/services/user/avatar.go index 79dfc76503..7ccb5e8c70 100644 --- a/services/user/avatar.go +++ b/services/user/avatar.go @@ -5,21 +5,19 @@ package user import ( "context" - "errors" "fmt" - "io" - "os" "forgejo.org/models/db" user_model "forgejo.org/models/user" "forgejo.org/modules/avatar" + "forgejo.org/modules/avatarstore" "forgejo.org/modules/log" "forgejo.org/modules/storage" ) // UploadAvatar saves custom avatar for user. func UploadAvatar(ctx context.Context, u *user_model.User, data []byte) error { - avatarData, err := avatar.ProcessAvatarImage(data) + avatarData, img, err := avatar.ProcessAvatarImage(data) if err != nil { return err } @@ -31,16 +29,20 @@ func UploadAvatar(ctx context.Context, u *user_model.User, data []byte) error { defer committer.Close() u.UseCustomAvatar = true + previousAvatar := u.Avatar u.Avatar = avatar.HashAvatar(u.ID, data) if err = user_model.UpdateUserCols(ctx, u, "use_custom_avatar", "avatar"); err != nil { return fmt.Errorf("updateUser: %w", err) } - if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error { - _, err := w.Write(avatarData) - return err - }); err != nil { - return fmt.Errorf("Failed to create dir %s: %w", u.CustomAvatarRelativePath(), err) + if err := avatarstore.StoreAvatar(u.CustomAvatarRelativePath(), avatarData, img, storage.Avatars); err != nil { + return fmt.Errorf("Failed to store avatar at %s: %w", u.CustomAvatarRelativePath(), err) + } + if len(previousAvatar) > 0 { + err := avatarstore.DeleteAvatar(previousAvatar, storage.Avatars) + if err != nil { + return err + } } return committer.Commit() @@ -60,14 +62,8 @@ func DeleteAvatar(ctx context.Context, u *user_model.User) error { } if hasAvatar { - if err := storage.Avatars.Delete(aPath); err != nil { - if !errors.Is(err, os.ErrNotExist) { - return fmt.Errorf("failed to remove %s: %w", aPath, err) - } - log.Warn("Deleting avatar %s but it doesn't exist", aPath) - } + return avatarstore.DeleteAvatar(aPath, storage.Avatars) } - return nil }) } diff --git a/services/user/avatar_test.go b/services/user/avatar_test.go index 17132a74ab..5ed7437f5a 100644 --- a/services/user/avatar_test.go +++ b/services/user/avatar_test.go @@ -5,14 +5,19 @@ package user import ( "bytes" + "fmt" "image" + "image/color" "image/png" "os" + "strings" "testing" "forgejo.org/models/db" "forgejo.org/models/unittest" user_model "forgejo.org/models/user" + "forgejo.org/modules/avatar" + "forgejo.org/modules/setting" "forgejo.org/modules/storage" "forgejo.org/modules/test" @@ -65,17 +70,70 @@ func TestUserDeleteAvatar(t *testing.T) { t.Run("Success", func(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) + defer test.MockVariableValue(&setting.Avatar.MaxOriginSize, 3)() user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) err := UploadAvatar(db.DefaultContext, user, buff.Bytes()) require.NoError(t, err) verification := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) assert.NotEmpty(t, verification.Avatar) + avatarID := strings.Clone(verification.Avatar) + + // Check that resized versions are stored in the cache + for _, size := range avatar.AllowedResizedAvatarSizes { + _, err := storage.Avatars.Stat(fmt.Sprintf("resized/%d/%s", size, avatarID)) + require.NoError(t, err) + } err = DeleteAvatar(db.DefaultContext, user) require.NoError(t, err) verification = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) assert.Empty(t, verification.Avatar) + + // The avatar is deleted from the storage + fi, err := storage.Avatars.Stat(avatarID) + require.Error(t, err) + assert.Nil(t, fi) + // All resized versions of the avatar are also deleted + for _, size := range avatar.AllowedResizedAvatarSizes { + fi, err := storage.Avatars.Stat(fmt.Sprintf("resized/%d/%s", size, avatarID)) + require.Error(t, err) + assert.Nil(t, fi) + } + }) +} + +func TestUserReplaceAvatar(t *testing.T) { + firstImage := image.NewRGBA(image.Rect(0, 0, 1, 1)) + var firstBuff bytes.Buffer + png.Encode(&firstBuff, firstImage) + secondImage := image.NewRGBA(image.Rect(0, 0, 2, 2)) + secondImage.Set(0, 0, color.White) + secondImage.Set(1, 1, color.Black) + var secondBuff bytes.Buffer + png.Encode(&secondBuff, secondImage) + + t.Run("Success", func(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + + err := UploadAvatar(db.DefaultContext, user, firstBuff.Bytes()) + require.NoError(t, err) + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + firstImageHash := user.Avatar + assert.NotEmpty(t, firstImageHash) + + err = UploadAvatar(db.DefaultContext, user, secondBuff.Bytes()) + require.NoError(t, err) + user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + secondImageHash := user.Avatar + assert.NotEmpty(t, secondImageHash) + assert.NotEqual(t, firstImageHash, secondImageHash) + + // The previous avatar is deleted from storage + fi, err := storage.Avatars.Stat(firstImageHash) + require.Error(t, err) + assert.Nil(t, fi) }) } diff --git a/tests/integration/doctor_avatars_test.go b/tests/integration/doctor_avatars_test.go new file mode 100644 index 0000000000..27528f7c6d --- /dev/null +++ b/tests/integration/doctor_avatars_test.go @@ -0,0 +1,101 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "bytes" + "fmt" + "image" + "image/png" + "testing" + + cmd "forgejo.org/cmd" + "forgejo.org/models/db" + "forgejo.org/models/repo" + "forgejo.org/models/unittest" + "forgejo.org/models/user" + "forgejo.org/modules/setting" + "forgejo.org/modules/storage" + "forgejo.org/modules/test" + "forgejo.org/tests" + + "github.com/stretchr/testify/require" +) + +func TestPrecomputeUserAvatars(t *testing.T) { + defer tests.PrepareTestEnv(t, 1)() + var err error + + // make the maximum uncached image size small, so that our test image is bigger than that + defer test.MockVariableValue(&setting.Avatar.MaxOriginSize, 3)() + + ctx := db.DefaultContext + + u := unittest.AssertExistsAndLoadBean(t, &user.User{ID: 2}) + // generate an avatar for this user + myImage := image.NewRGBA(image.Rect(0, 0, 1024, 1024)) + var buff bytes.Buffer + png.Encode(&buff, myImage) + // store the avatar, but only the original (not the resized versions) + avatarPath := "some_id" + storage.Avatars.Save(avatarPath, bytes.NewReader(buff.Bytes()), -1) + u.UseCustomAvatar = true + u.Avatar = avatarPath + err = user.UpdateUserCols(ctx, u, "use_custom_avatar", "avatar") + require.NoError(t, err) + + // run the doctor command + err = cmd.RunAvatarResize(ctx, true, false) + require.NoError(t, err) + + // the resized version of the avatar is now stored in the cache + _, err = storage.Avatars.Stat(fmt.Sprintf("resized/64/%s", avatarPath)) + require.NoError(t, err) +} + +func TestPrecomputeRepoAvatars(t *testing.T) { + defer tests.PrepareTestEnv(t, 1)() + var err error + // make the maximum uncached image size small, so that our test image is bigger than that + defer test.MockVariableValue(&setting.Avatar.MaxOriginSize, 3)() + + ctx := db.DefaultContext + + u := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 2}) + // generate an avatar for this repo + myImage := image.NewRGBA(image.Rect(0, 0, 1024, 1024)) + var buff bytes.Buffer + png.Encode(&buff, myImage) + // store the avatar, but only the original (not the resized versions) + avatarPath := "some_id" + storage.RepoAvatars.Save(avatarPath, bytes.NewReader(buff.Bytes()), -1) + u.Avatar = avatarPath + err = repo.UpdateRepositoryCols(ctx, u, "avatar") + require.NoError(t, err) + // make sure there is no resized version of the avatar in the storage yet + storage.RepoAvatars.Delete(fmt.Sprintf("resized/64/%s", avatarPath)) + _, err = storage.RepoAvatars.Stat(fmt.Sprintf("resized/64/%s", avatarPath)) + require.Error(t, err) + + // run the doctor command + err = cmd.RunAvatarResize(ctx, false, true) + require.NoError(t, err) + + // the resized version of the avatar is now stored in the cache + _, err = storage.RepoAvatars.Stat(fmt.Sprintf("resized/64/%s", avatarPath)) + require.NoError(t, err) +} + +func TestPrecomputeAvatarsWithoutArgument(t *testing.T) { + defer tests.PrepareTestEnv(t, 1)() + var err error + + ctx := db.DefaultContext + + // run the doctor command + err = cmd.RunAvatarResize(ctx, false, false) + + // there is an error because we didn't specify which avatars to resize + require.Error(t, err) +} diff --git a/tests/integration/doctor_storage_test.go b/tests/integration/doctor_storage_test.go new file mode 100644 index 0000000000..526871427b --- /dev/null +++ b/tests/integration/doctor_storage_test.go @@ -0,0 +1,139 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "bytes" + "context" + "fmt" + "image" + "image/color" + "image/png" + "testing" + + "forgejo.org/models/db" + "forgejo.org/models/repo" + "forgejo.org/models/unittest" + "forgejo.org/models/user" + "forgejo.org/modules/log" + "forgejo.org/modules/setting" + "forgejo.org/modules/storage" + "forgejo.org/modules/test" + doctor "forgejo.org/services/doctor" + repo_service "forgejo.org/services/repository" + user_service "forgejo.org/services/user" + "forgejo.org/tests" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func generateUserAvatar(ctx context.Context, t *testing.T, user *user.User) string { + err := user_service.UploadAvatar(ctx, user, generateAvatar(user.ID)) + require.NoError(t, err) + assert.NotEmpty(t, user.Avatar) + return user.CustomAvatarRelativePath() +} + +func generateRepoAvatar(ctx context.Context, t *testing.T, repo *repo.Repository) string { + err := repo_service.UploadAvatar(ctx, repo, generateAvatar(repo.ID)) + require.NoError(t, err) + assert.NotEmpty(t, repo.Avatar) + return repo.CustomAvatarRelativePath() +} + +func generateAvatar(objectID int64) []byte { + avatar := image.NewRGBA(image.Rect(0, 0, 1024, 1024)) + // make the avatar distinctive for the user by setting a pixel based on the object id + avatar.SetRGBA(3, 3, color.RGBA{R: uint8(objectID % 255), G: uint8(objectID / 255 % 255), B: 55, A: 66}) + var buff bytes.Buffer + png.Encode(&buff, avatar) + return buff.Bytes() +} + +func TestRemoveUnusedUserAvatars(t *testing.T) { + defer tests.PrepareTestEnv(t, 1)() + // make the maximum uncached image size small, so that our test image is bigger than that + defer test.MockVariableValue(&setting.Avatar.MaxOriginSize, 3)() + var err error + + ctx := db.DefaultContext + + user2 := unittest.AssertExistsAndLoadBean(t, &user.User{ID: 2}) + user3 := unittest.AssertExistsAndLoadBean(t, &user.User{ID: 3}) + avatarPathUser2 := generateUserAvatar(ctx, t, user2) + avatarPathUser3 := generateUserAvatar(ctx, t, user3) + + // disconnect the avatar from user2, keeping the avatar for user3 + user2.Avatar = "" + err = user.UpdateUserCols(ctx, user2, "avatar") + require.NoError(t, err) + + // make sure both avatars are still stored as a file + _, err = storage.Avatars.Stat(avatarPathUser2) + require.NoError(t, err) + _, err = storage.Avatars.Stat(avatarPathUser3) + require.NoError(t, err) + // the downscaled versions are also stored + _, err = storage.Avatars.Stat(fmt.Sprintf("resized/64/%s", avatarPathUser2)) + require.NoError(t, err) + _, err = storage.Avatars.Stat(fmt.Sprintf("resized/64/%s", avatarPathUser3)) + require.NoError(t, err) + + doctor.CheckStorage(&doctor.CheckStorageOptions{Avatars: true})(ctx, log.GetLogger("doctor"), true) + + // the avatar for user2 is no longer stored + _, err = storage.Avatars.Stat(avatarPathUser2) + require.Error(t, err) + // its downscaled versions are not stored either + _, err = storage.Avatars.Stat(fmt.Sprintf("resized/64/%s", avatarPathUser2)) + require.Error(t, err) + // the avatar for user3 is still stored stored + _, err = storage.Avatars.Stat(avatarPathUser3) + require.NoError(t, err) + // the downscaled versions are also stored + _, err = storage.Avatars.Stat(fmt.Sprintf("resized/64/%s", avatarPathUser3)) + require.NoError(t, err) +} + +func TestRemoveUnusedRepoAvatars(t *testing.T) { + defer tests.PrepareTestEnv(t, 1)() + // make the maximum uncached image size small, so that our test image is bigger than that + defer test.MockVariableValue(&setting.Avatar.MaxOriginSize, 3)() + var err error + + ctx := db.DefaultContext + + repo2 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 2}) + repo3 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 3}) + avatarPathRepo2 := generateRepoAvatar(ctx, t, repo2) + avatarPathRepo3 := generateRepoAvatar(ctx, t, repo3) + + // disconnect the avatar from repo2, but keep it for repo3 + repo2.Avatar = "" + err = repo.UpdateRepositoryCols(ctx, repo2, "avatar") + require.NoError(t, err) + + // make sure the avatar is still stored as a file + _, err = storage.RepoAvatars.Stat(avatarPathRepo2) + require.NoError(t, err) + // the downscaled versions are also stored + _, err = storage.RepoAvatars.Stat(fmt.Sprintf("resized/64/%s", avatarPathRepo2)) + require.NoError(t, err) + + doctor.CheckStorage(&doctor.CheckStorageOptions{RepoAvatars: true})(ctx, log.GetLogger("doctor"), true) + + // the avatar for repo2 is no longer stored + _, err = storage.RepoAvatars.Stat(avatarPathRepo2) + require.Error(t, err) + // its downscaled versions are not stored either + _, err = storage.RepoAvatars.Stat(fmt.Sprintf("resized/64/%s", avatarPathRepo2)) + require.Error(t, err) + // the avatar for repo3 is still stored + _, err = storage.RepoAvatars.Stat(avatarPathRepo3) + require.NoError(t, err) + // its downscaled versions are also still stored + _, err = storage.RepoAvatars.Stat(fmt.Sprintf("resized/64/%s", avatarPathRepo3)) + require.NoError(t, err) +} diff --git a/tests/integration/user_avatar_test.go b/tests/integration/user_avatar_test.go index b6ccfef9a2..a07ed989ee 100644 --- a/tests/integration/user_avatar_test.go +++ b/tests/integration/user_avatar_test.go @@ -6,6 +6,7 @@ package integration import ( "bytes" "fmt" + "image" "image/png" "io" "mime/multipart" @@ -17,6 +18,8 @@ import ( "forgejo.org/models/unittest" user_model "forgejo.org/models/user" "forgejo.org/modules/avatar" + "forgejo.org/modules/setting" + "forgejo.org/modules/test" "forgejo.org/tests" "github.com/stretchr/testify/assert" @@ -25,6 +28,9 @@ import ( func TestUserAvatar(t *testing.T) { defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.Avatar.Storage.Type, setting.LocalStorageType)() + // make the maximum uncached image size small, so that our test image is bigger than that + defer test.MockVariableValue(&setting.Avatar.MaxOriginSize, 3)() user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the repo3, is an org seed := user2.Email @@ -82,7 +88,34 @@ func TestUserAvatar(t *testing.T) { resp := MakeRequest(t, req, http.StatusSeeOther) assert.Equal(t, fmt.Sprintf("/avatars/%s", user2.Avatar), resp.Header().Get("location")) - // Can't test if the response matches because the image is re-generated on upload but checking that this at least doesn't give a 404 should be enough. + req = NewRequest(t, "GET", resp.Header().Get("location")) + resp = MakeRequest(t, req, http.StatusOK) + + // check that it's a valid image with the expected dimensions + respImg, _, err := image.Decode(resp.Body) + require.NoError(t, err) + assert.Equal(t, 512, respImg.Bounds().Dx()) + assert.Equal(t, 512, respImg.Bounds().Dy()) + + // request an avatar that doesn't exist + req = NewRequest(t, "GET", "/avatars/not_found") + MakeRequest(t, req, http.StatusNotFound) + + // request an avatar that exists, but with an invalid size + req = NewRequest(t, "GET", user2.AvatarLinkWithSize(db.DefaultContext, 0)+"?size=123456") + MakeRequest(t, req, http.StatusNotFound) + + // request an avatar with a correct size + req = NewRequest(t, "GET", user2.AvatarLinkWithSize(db.DefaultContext, 64)) + resp = MakeRequest(t, req, http.StatusOK) + respImg, _, err = image.Decode(resp.Body) + require.NoError(t, err) + assert.Equal(t, 64, respImg.Bounds().Dx()) + assert.Equal(t, 64, respImg.Bounds().Dy()) + + // request a resized avatar using its internal storage path: not found + req = NewRequest(t, "GET", fmt.Sprintf("/avatars/resized/64/%s", user2.Avatar)) + MakeRequest(t, req, http.StatusNotFound) } func TestAvatarAnchorDestination(t *testing.T) {