From 58a03e9fadb345de5653345c2a68ecfd0750940a Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Sun, 19 May 2024 12:58:39 +0800
Subject: [PATCH] Fix bug on avatar (#31008)

Co-authored-by: silverwind <me@silverwind.io>
---
 routers/api/v1/org/avatar.go  |  2 ++
 routers/api/v1/user/avatar.go |  2 ++
 services/user/avatar.go       | 32 +++++++++++++++++++++-----------
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/routers/api/v1/org/avatar.go b/routers/api/v1/org/avatar.go
index e34c68dfc9..f11eb6c1cd 100644
--- 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 f912296228..30ccb63587 100644
--- 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 2d6c3faf9a..3f87466eaa 100644
--- 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)
-		}
-	}
 
-	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
+	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)
+		}
+
+		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
+	})
 }