refactor(db): move some methods off `user.go` (#7199)

13 files changed, 180 insertions, 103 deletions

diff --git a/internal/cmd/web.go b/internal/cmd/web.go
index 1689afef..3be1db06 100644
--- a/internal/cmd/web.go
+++ b/internal/cmd/web.go
@@ -99,7 +99,7 @@ func newMacaron() *macaron.Macaron {
 		conf.Picture.AvatarUploadPath,
 		macaron.StaticOptions{
 			ETag:        true,
-			Prefix:      conf.UsersAvatarURLPath,
+			Prefix:      conf.UsersAvatarPathPrefix,
 			SkipLogging: conf.Server.DisableRouterLog,
 		},
 	))
diff --git a/internal/conf/mocks.go b/internal/conf/mocks.go
index af5b1bd7..2c10c1ce 100644
--- a/internal/conf/mocks.go
+++ b/internal/conf/mocks.go
@@ -55,3 +55,11 @@ func SetMockUI(t *testing.T, opts UIOpts) {
 		UI = before
 	})
 }
+
+func SetMockPicture(t *testing.T, opts PictureOpts) {
+	before := Picture
+	Picture = opts
+	t.Cleanup(func() {
+		Picture = before
+	})
+}
diff --git a/internal/conf/static.go b/internal/conf/static.go
index 8132547f..a514f7a6 100644
--- a/internal/conf/static.go
+++ b/internal/conf/static.go
@@ -12,12 +12,6 @@ import (
 	"github.com/gogs/go-libravatar"
 )
 
-const (
-	// UsersAvatarURLPath is used to identify whether a URL is to access user
-	// avatars.
-	UsersAvatarURLPath = "avatars"
-)
-
 // ℹ️ README: This file contains static values that should only be set at initialization time.
 //
 // ⚠️ WARNING: After changing any options, do not forget to update template of
@@ -132,18 +126,6 @@ var (
 		FormatLayout string `ini:"-"` // Actual layout of the Format.
 	}
 
-	// Picture settings
-	Picture struct {
-		AvatarUploadPath           string
-		RepositoryAvatarUploadPath string
-		GravatarSource             string
-		DisableGravatar            bool
-		EnableFederatedAvatar      bool
-
-		// Derived from other static values
-		LibravatarService *libravatar.Libravatar `ini:"-"` // Initialized client for federated avatar.
-	}
-
 	// Mirror settings
 	Mirror struct {
 		DefaultInterval int
@@ -408,6 +390,20 @@ type UIOpts struct {
 // UI settings
 var UI UIOpts
 
+type PictureOpts struct {
+	AvatarUploadPath           string
+	RepositoryAvatarUploadPath string
+	GravatarSource             string
+	DisableGravatar            bool
+	EnableFederatedAvatar      bool
+
+	// Derived from other static values
+	LibravatarService *libravatar.Libravatar `ini:"-"` // Initialized client for federated avatar.
+}
+
+// Picture settings
+var Picture PictureOpts
+
 type i18nConf struct {
 	Langs     []string          `delim:","`
 	Names     []string          `delim:","`
@@ -448,3 +444,11 @@ var (
 	UsePostgreSQL bool
 	UseMSSQL      bool
 )
+
+// UsersAvatarPathPrefix is the path prefix to user avatars.
+const UsersAvatarPathPrefix = "avatars"
+
+// UserDefaultAvatarURLPath returns the URL path of the default user avatar.
+func UserDefaultAvatarURLPath() string {
+	return Server.Subpath + "/img/avatar_default.png"
+}
diff --git a/internal/db/actions.go b/internal/db/actions.go
index b7f0cca9..74c15291 100644
--- a/internal/db/actions.go
+++ b/internal/db/actions.go
@@ -954,7 +954,7 @@ func (pcs *PushCommits) AvatarLink(email string) string {
 				log.Error("Failed to get user [email: %s]: %v", email, err)
 			}
 		} else {
-			pcs.avatars[email] = u.RelAvatarLink()
+			pcs.avatars[email] = u.AvatarURLPath()
 		}
 	}
 
diff --git a/internal/db/org.go b/internal/db/org.go
index c5c55f44..93350746 100644
--- a/internal/db/org.go
+++ b/internal/db/org.go
@@ -14,6 +14,7 @@ import (
 	"xorm.io/xorm"
 
 	"gogs.io/gogs/internal/errutil"
+	"gogs.io/gogs/internal/userutil"
 )
 
 var ErrOrgNotExist = errors.New("Organization does not exist")
@@ -131,7 +132,7 @@ func CreateOrganization(org, owner *User) (err error) {
 	if _, err = sess.Insert(org); err != nil {
 		return fmt.Errorf("insert organization: %v", err)
 	}
-	_ = org.GenerateRandomAvatar()
+	_ = userutil.GenerateRandomAvatar(org.ID, org.Name, org.Email)
 
 	// Add initial creator to organization and owner team.
 	if _, err = sess.Insert(&OrgUser{
diff --git a/internal/db/user.go b/internal/db/user.go
index 11e4c4a6..48444b1e 100644
--- a/internal/db/user.go
+++ b/internal/db/user.go
@@ -34,6 +34,7 @@ import (
 	"gogs.io/gogs/internal/errutil"
 	"gogs.io/gogs/internal/strutil"
 	"gogs.io/gogs/internal/tool"
+	"gogs.io/gogs/internal/userutil"
 )
 
 // TODO(unknwon): Delete me once refactoring is done.
@@ -60,75 +61,6 @@ func (u *User) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
-// CustomAvatarPath returns user custom avatar file path.
-func (u *User) CustomAvatarPath() string {
-	return filepath.Join(conf.Picture.AvatarUploadPath, com.ToStr(u.ID))
-}
-
-// GenerateRandomAvatar generates a random avatar for user.
-func (u *User) GenerateRandomAvatar() error {
-	seed := u.Email
-	if seed == "" {
-		seed = u.Name
-	}
-
-	img, err := avatar.RandomImage([]byte(seed))
-	if err != nil {
-		return fmt.Errorf("RandomImage: %v", err)
-	}
-	if err = os.MkdirAll(filepath.Dir(u.CustomAvatarPath()), os.ModePerm); err != nil {
-		return fmt.Errorf("MkdirAll: %v", err)
-	}
-	fw, err := os.Create(u.CustomAvatarPath())
-	if err != nil {
-		return fmt.Errorf("Create: %v", err)
-	}
-	defer fw.Close()
-
-	if err = png.Encode(fw, img); err != nil {
-		return fmt.Errorf("Encode: %v", err)
-	}
-
-	log.Info("New random avatar created: %d", u.ID)
-	return nil
-}
-
-// RelAvatarLink returns relative avatar link to the site domain,
-// which includes app sub-url as prefix. However, it is possible
-// to return full URL if user enables Gravatar-like service.
-func (u *User) RelAvatarLink() string {
-	defaultImgUrl := conf.Server.Subpath + "/img/avatar_default.png"
-	if u.ID == -1 {
-		return defaultImgUrl
-	}
-
-	switch {
-	case u.UseCustomAvatar:
-		if !com.IsExist(u.CustomAvatarPath()) {
-			return defaultImgUrl
-		}
-		return fmt.Sprintf("%s/%s/%d", conf.Server.Subpath, conf.UsersAvatarURLPath, u.ID)
-	case conf.Picture.DisableGravatar:
-		if !com.IsExist(u.CustomAvatarPath()) {
-			if err := u.GenerateRandomAvatar(); err != nil {
-				log.Error("GenerateRandomAvatar: %v", err)
-			}
-		}
-
-		return fmt.Sprintf("%s/%s/%d", conf.Server.Subpath, conf.UsersAvatarURLPath, u.ID)
-	}
-	return tool.AvatarLink(u.AvatarEmail)
-}
-
-// AvatarLink returns user avatar absolute link.
-func (u *User) AvatarLink() string {
-	link := u.RelAvatarLink()
-	if link[0] == '/' && link[1] != '/' {
-		return conf.Server.ExternalURL + strings.TrimPrefix(link, conf.Server.Subpath)[1:]
-	}
-	return link
-}
-
 // User.GetFollowers returns range of user's followers.
 func (u *User) GetFollowers(page int) ([]*User, error) {
 	users := make([]*User, 0, ItemsPerPage)
@@ -188,7 +120,7 @@ func (u *User) UploadAvatar(data []byte) error {
 	}
 
 	_ = os.MkdirAll(conf.Picture.AvatarUploadPath, os.ModePerm)
-	fw, err := os.Create(u.CustomAvatarPath())
+	fw, err := os.Create(userutil.CustomAvatarPath(u.ID))
 	if err != nil {
 		return fmt.Errorf("create custom avatar directory: %v", err)
 	}
@@ -204,8 +136,9 @@ func (u *User) UploadAvatar(data []byte) error {
 
 // DeleteAvatar deletes the user's custom avatar.
 func (u *User) DeleteAvatar() error {
-	log.Trace("DeleteAvatar [%d]: %s", u.ID, u.CustomAvatarPath())
-	if err := os.Remove(u.CustomAvatarPath()); err != nil {
+	avatarPath := userutil.CustomAvatarPath(u.ID)
+	log.Trace("DeleteAvatar [%d]: %s", u.ID, avatarPath)
+	if err := os.Remove(avatarPath); err != nil {
 		return err
 	}
 
@@ -705,7 +638,7 @@ func deleteUser(e *xorm.Session, u *User) error {
 	//	so just keep error logs of those operations.
 
 	_ = os.RemoveAll(UserPath(u.Name))
-	_ = os.Remove(u.CustomAvatarPath())
+	_ = os.Remove(userutil.CustomAvatarPath(u.ID))
 
 	return nil
 }
diff --git a/internal/db/users.go b/internal/db/users.go
index 1ae50064..bc57f317 100644
--- a/internal/db/users.go
+++ b/internal/db/users.go
@@ -14,11 +14,15 @@ import (
 	api "github.com/gogs/go-gogs-client"
 	"github.com/pkg/errors"
 	"gorm.io/gorm"
+	log "unknwon.dev/clog/v2"
 
 	"gogs.io/gogs/internal/auth"
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/cryptoutil"
 	"gogs.io/gogs/internal/errutil"
+	"gogs.io/gogs/internal/osutil"
+	"gogs.io/gogs/internal/tool"
+	"gogs.io/gogs/internal/userutil"
 )
 
 // UsersStore is the persistent interface for users.
@@ -432,7 +436,7 @@ func (u *User) APIFormat() *api.User {
 		Login:     u.Name,
 		FullName:  u.FullName,
 		Email:     u.Email,
-		AvatarUrl: u.AvatarLink(),
+		AvatarUrl: u.AvatarURL(),
 	}
 }
 
@@ -474,7 +478,7 @@ func (u *User) HomeURLPath() string {
 	return conf.Server.Subpath + "/" + u.Name
 }
 
-// HTMLURL returns the HTML URL to the user or organization home page.
+// HTMLURL returns the full URL to the user or organization home page.
 //
 // TODO(unknwon): This is also used in templates, which should be fixed by
 // having a dedicated type `template.User` and move this to the "userutil"
@@ -482,3 +486,47 @@ func (u *User) HomeURLPath() string {
 func (u *User) HTMLURL() string {
 	return conf.Server.ExternalURL + u.Name
 }
+
+// AvatarURLPath returns the URL path to the user or organization avatar. If the
+// user enables Gravatar-like service, then an external URL will be returned.
+//
+// TODO(unknwon): This is also used in templates, which should be fixed by
+// having a dedicated type `template.User` and move this to the "userutil"
+// package.
+func (u *User) AvatarURLPath() string {
+	defaultURLPath := conf.UserDefaultAvatarURLPath()
+	if u.ID <= 0 {
+		return defaultURLPath
+	}
+
+	hasCustomAvatar := osutil.IsFile(userutil.CustomAvatarPath(u.ID))
+	switch {
+	case u.UseCustomAvatar:
+		if !hasCustomAvatar {
+			return defaultURLPath
+		}
+		return fmt.Sprintf("%s/%s/%d", conf.Server.Subpath, conf.UsersAvatarPathPrefix, u.ID)
+	case conf.Picture.DisableGravatar:
+		if !hasCustomAvatar {
+			if err := userutil.GenerateRandomAvatar(u.ID, u.Name, u.Email); err != nil {
+				log.Error("Failed to generate random avatar [user_id: %d]: %v", u.ID, err)
+			}
+		}
+		return fmt.Sprintf("%s/%s/%d", conf.Server.Subpath, conf.UsersAvatarPathPrefix, u.ID)
+	}
+	return tool.AvatarLink(u.AvatarEmail)
+}
+
+// AvatarURL returns the full URL to the user or organization avatar. If the
+// user enables Gravatar-like service, then an external URL will be returned.
+//
+// TODO(unknwon): This is also used in templates, which should be fixed by
+// having a dedicated type `template.User` and move this to the "userutil"
+// package.
+func (u *User) AvatarURL() string {
+	link := u.AvatarURLPath()
+	if link[0] == '/' && link[1] != '/' {
+		return conf.Server.ExternalURL + strings.TrimPrefix(link, conf.Server.Subpath)[1:]
+	}
+	return link
+}
diff --git a/internal/route/api/v1/convert/convert.go b/internal/route/api/v1/convert/convert.go
index bf5aabbd..04d4df33 100644
--- a/internal/route/api/v1/convert/convert.go
+++ b/internal/route/api/v1/convert/convert.go
@@ -120,7 +120,7 @@ func ToDeployKey(apiLink string, key *db.DeployKey) *api.DeployKey {
 func ToOrganization(org *db.User) *api.Organization {
 	return &api.Organization{
 		ID:          org.ID,
-		AvatarUrl:   org.AvatarLink(),
+		AvatarUrl:   org.AvatarURL(),
 		UserName:    org.Name,
 		FullName:    org.FullName,
 		Description: org.Description,
diff --git a/internal/route/api/v1/user/user.go b/internal/route/api/v1/user/user.go
index 2b26a282..5852f660 100644
--- a/internal/route/api/v1/user/user.go
+++ b/internal/route/api/v1/user/user.go
@@ -40,7 +40,7 @@ func Search(c *context.APIContext) {
 		results[i] = &api.User{
 			ID:        users[i].ID,
 			UserName:  users[i].Name,
-			AvatarUrl: users[i].AvatarLink(),
+			AvatarUrl: users[i].AvatarURL(),
 			FullName:  markup.Sanitize(users[i].FullName),
 		}
 		if c.IsLogged {
diff --git a/internal/route/user/home.go b/internal/route/user/home.go
index a14572ed..5a2ebf8e 100644
--- a/internal/route/user/home.go
+++ b/internal/route/user/home.go
@@ -82,7 +82,7 @@ func retrieveFeeds(c *context.Context, ctxUser *db.User, userID int64, isProfile
 				c.Error(err, "get user by name")
 				return
 			}
-			unameAvatars[act.ActUserName] = u.RelAvatarLink()
+			unameAvatars[act.ActUserName] = u.AvatarURLPath()
 		}
 
 		act.ActAvatar = unameAvatars[act.ActUserName]
diff --git a/internal/route/user/setting.go b/internal/route/user/setting.go
index 8c0e87c1..54771abf 100644
--- a/internal/route/user/setting.go
+++ b/internal/route/user/setting.go
@@ -27,6 +27,7 @@ import (
 	"gogs.io/gogs/internal/email"
 	"gogs.io/gogs/internal/form"
 	"gogs.io/gogs/internal/tool"
+	"gogs.io/gogs/internal/userutil"
 )
 
 const (
@@ -144,8 +145,8 @@ func UpdateAvatarSetting(c *context.Context, f form.Avatar, ctxUser *db.User) er
 	} else {
 		// No avatar is uploaded but setting has been changed to enable,
 		// generate a random one when needed.
-		if ctxUser.UseCustomAvatar && !com.IsFile(ctxUser.CustomAvatarPath()) {
-			if err := ctxUser.GenerateRandomAvatar(); err != nil {
+		if ctxUser.UseCustomAvatar && !com.IsFile(userutil.CustomAvatarPath(ctxUser.ID)) {
+			if err := userutil.GenerateRandomAvatar(ctxUser.ID, ctxUser.Name, ctxUser.Email); err != nil {
 				log.Error("generate random avatar [%d]: %v", ctxUser.ID, err)
 			}
 		}
diff --git a/internal/userutil/userutil.go b/internal/userutil/userutil.go
index 87b8f15b..d5c74325 100644
--- a/internal/userutil/userutil.go
+++ b/internal/userutil/userutil.go
@@ -7,8 +7,15 @@ package userutil
 import (
 	"encoding/hex"
 	"fmt"
+	"image/png"
+	"os"
+	"path/filepath"
+	"strconv"
 	"strings"
 
+	"github.com/pkg/errors"
+
+	"gogs.io/gogs/internal/avatar"
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/tool"
 )
@@ -23,9 +30,9 @@ func DashboardURLPath(name string, isOrganization bool) string {
 
 // GenerateActivateCode generates an activate code based on user information and
 // the given email.
-func GenerateActivateCode(id int64, email, name, password, rands string) string {
+func GenerateActivateCode(userID int64, email, name, password, rands string) string {
 	code := tool.CreateTimeLimitCode(
-		fmt.Sprintf("%d%s%s%s%s", id, email, strings.ToLower(name), password, rands),
+		fmt.Sprintf("%d%s%s%s%s", userID, email, strings.ToLower(name), password, rands),
 		conf.Auth.ActivateCodeLives,
 		nil,
 	)
@@ -34,3 +41,39 @@ func GenerateActivateCode(id int64, email, name, password, rands string) string
 	code += hex.EncodeToString([]byte(strings.ToLower(name)))
 	return code
 }
+
+// CustomAvatarPath returns the absolute path of the user custom avatar file.
+func CustomAvatarPath(userID int64) string {
+	return filepath.Join(conf.Picture.AvatarUploadPath, strconv.FormatInt(userID, 10))
+}
+
+// GenerateRandomAvatar generates a random avatar and stores to local file
+// system using given user information.
+func GenerateRandomAvatar(userID int64, name, email string) error {
+	seed := email
+	if seed == "" {
+		seed = name
+	}
+
+	img, err := avatar.RandomImage([]byte(seed))
+	if err != nil {
+		return errors.Wrap(err, "generate random image")
+	}
+
+	avatarPath := CustomAvatarPath(userID)
+	err = os.MkdirAll(filepath.Dir(avatarPath), os.ModePerm)
+	if err != nil {
+		return errors.Wrap(err, "create avatar directory")
+	}
+
+	f, err := os.Create(avatarPath)
+	if err != nil {
+		return errors.Wrap(err, "create avatar file")
+	}
+	defer func() { _ = f.Close() }()
+
+	if err = png.Encode(f, img); err != nil {
+		return errors.Wrap(err, "encode avatar image to file")
+	}
+	return nil
+}
diff --git a/internal/userutil/userutil_test.go b/internal/userutil/userutil_test.go
index a62363a5..e90c9235 100644
--- a/internal/userutil/userutil_test.go
+++ b/internal/userutil/userutil_test.go
@@ -5,11 +5,15 @@
 package userutil
 
 import (
+	"os"
+	"runtime"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"gogs.io/gogs/internal/conf"
+	"gogs.io/gogs/internal/osutil"
 	"gogs.io/gogs/internal/tool"
 )
 
@@ -38,3 +42,38 @@ func TestGenerateActivateCode(t *testing.T) {
 	got := tool.VerifyTimeLimitCode("1alice@example.comalice123456rands", conf.Auth.ActivateCodeLives, code[:tool.TIME_LIMIT_CODE_LENGTH])
 	assert.True(t, got)
 }
+
+func TestCustomAvatarPath(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Skipping testing on Windows")
+		return
+	}
+
+	conf.SetMockPicture(t,
+		conf.PictureOpts{
+			AvatarUploadPath: "data/avatars",
+		},
+	)
+
+	got := CustomAvatarPath(1)
+	want := "data/avatars/1"
+	assert.Equal(t, want, got)
+}
+
+func TestGenerateRandomAvatar(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Skipping testing on Windows")
+		return
+	}
+
+	conf.SetMockPicture(t,
+		conf.PictureOpts{
+			AvatarUploadPath: os.TempDir(),
+		},
+	)
+
+	err := GenerateRandomAvatar(1, "alice", "alice@example.com")
+	require.NoError(t, err)
+	got := osutil.IsFile(CustomAvatarPath(1))
+	assert.True(t, got)
+}