Fix bug on avatar (#31008) (#31019)

Backport #31008 by @lunny

Extract from #30995

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: silverwind <me@silverwind.io>

diff --git a/routers/api/v1/org/avatar.go b/routers/api/v1/org/avatar.go
index e34c68dfc9da1cf5a55295d10c7b3faf7f6538e4..f11eb6c1cd135e88f8475045cd7d7175d7d1d314 100644 (file)
--- a/routers/api/v1/org/avatar.go
+++ b/routers/api/v1/org/avatar.go
@@ -46,6 +46,7 @@ func UpdateAvatar(ctx *context.APIContext) {
        err = user_service.UploadAvatar(ctx, ctx.Org.Organization.AsUser(), content)
        if err != nil {
                ctx.Error(http.StatusInternalServerError, "UploadAvatar", err)
+               return
        }
 
        ctx.Status(http.StatusNoContent)
@@ -72,6 +73,7 @@ func DeleteAvatar(ctx *context.APIContext) {
        err := user_service.DeleteAvatar(ctx, ctx.Org.Organization.AsUser())
        if err != nil {
                ctx.Error(http.StatusInternalServerError, "DeleteAvatar", err)
+               return
        }
 
        ctx.Status(http.StatusNoContent)

diff --git a/routers/api/v1/user/avatar.go b/routers/api/v1/user/avatar.go
index f912296228e7d64e80dd046723edb8b966c9fda1..30ccb63587b9dd0480df57b5172c7468ea476f57 100644 (file)
--- a/routers/api/v1/user/avatar.go
+++ b/routers/api/v1/user/avatar.go
@@ -39,6 +39,7 @@ func UpdateAvatar(ctx *context.APIContext) {
        err = user_service.UploadAvatar(ctx, ctx.Doer, content)
        if err != nil {
                ctx.Error(http.StatusInternalServerError, "UploadAvatar", err)
+               return
        }
 
        ctx.Status(http.StatusNoContent)
@@ -57,6 +58,7 @@ func DeleteAvatar(ctx *context.APIContext) {
        err := user_service.DeleteAvatar(ctx, ctx.Doer)
        if err != nil {
                ctx.Error(http.StatusInternalServerError, "DeleteAvatar", err)
+               return
        }
 
        ctx.Status(http.StatusNoContent)

diff --git a/services/user/avatar.go b/services/user/avatar.go
index 2d6c3faf9a509f8aa5e792fa85f5ffa037591faf..3f87466eaa6df3dd08d7f0fc311c02f73bf9168a 100644 (file)
--- a/services/user/avatar.go
+++ b/services/user/avatar.go
@@ -5,8 +5,10 @@ package user
 
 import (
        "context"
+       "errors"
        "fmt"
        "io"
+       "os"
 
        "code.gitea.io/gitea/models/db"
        user_model "code.gitea.io/gitea/models/user"
@@ -48,16 +50,24 @@ func UploadAvatar(ctx context.Context, u *user_model.User, data []byte) error {
 func DeleteAvatar(ctx context.Context, u *user_model.User) error {
        aPath := u.CustomAvatarRelativePath()
        log.Trace("DeleteAvatar[%d]: %s", u.ID, aPath)
-       if len(u.Avatar) > 0 {
-               if err := storage.Avatars.Delete(aPath); err != nil {
-                       return fmt.Errorf("Failed to remove %s: %w", aPath, err)
+
+       return db.WithTx(ctx, func(ctx context.Context) error {
+               hasAvatar := len(u.Avatar) > 0
+               u.UseCustomAvatar = false
+               u.Avatar = ""
+               if _, err := db.GetEngine(ctx).ID(u.ID).Cols("avatar, use_custom_avatar").Update(u); err != nil {
+                       return fmt.Errorf("DeleteAvatar: %w", err)
                }
-       }
 
-       u.UseCustomAvatar = false
-       u.Avatar = ""
-       if _, err := db.GetEngine(ctx).ID(u.ID).Cols("avatar, use_custom_avatar").Update(u); err != nil {
-               return fmt.Errorf("DeleteAvatar: %w", err)
-       }
-       return nil
+               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 nil
+       })
 }