Avatar refactor, move avatar code from `models` to `models.avatars`, remove duplicated code (#17123)

Why this refactor

The goal is to move most files from `models` package to `models.xxx` package. Many models depend on avatar model, so just move this first.

And the existing logic is not clear, there are too many function like `AvatarLink`, `RelAvatarLink`, `SizedRelAvatarLink`, `SizedAvatarLink`, `MakeFinalAvatarURL`, `HashedAvatarLink`, etc. This refactor make everything clear:

* user.AvatarLink()
* user.AvatarLinkWithSize(size)
* avatars.GenerateEmailAvatarFastLink(email, size)
* avatars.GenerateEmailAvatarFinalLink(email, size)

And many duplicated code are deleted in route handler, the handler and the model share the same avatar logic now.

4 files changed, 43 insertions, 24 deletions

diff --git a/modules/httpcache/httpcache.go b/modules/httpcache/httpcache.go
index f5e3906be6..35d4e6dfd8 100644
--- a/modules/httpcache/httpcache.go
+++ b/modules/httpcache/httpcache.go
@@ -16,12 +16,17 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 )
 
-// GetCacheControl returns a suitable "Cache-Control" header value
-func GetCacheControl() string {
-	if !setting.IsProd() {
-		return "no-store"
+// AddCacheControlToHeader adds suitable cache-control headers to response
+func AddCacheControlToHeader(h http.Header, d time.Duration) {
+	if setting.IsProd() {
+		h.Set("Cache-Control", "private, max-age="+strconv.Itoa(int(d.Seconds())))
+	} else {
+		h.Set("Cache-Control", "no-store")
+		// to remind users they are using non-prod setting.
+		// some users may be confused by "Cache-Control: no-store" in their setup if they did wrong to `RUN_MODE` in `app.ini`.
+		h.Add("X-Gitea-Debug", "RUN_MODE="+setting.RunMode)
+		h.Add("X-Gitea-Debug", "CacheControl=no-store")
 	}
-	return "private, max-age=" + strconv.FormatInt(int64(setting.StaticCacheTime.Seconds()), 10)
 }
 
 // generateETag generates an ETag based on size, filename and file modification time
@@ -32,7 +37,7 @@ func generateETag(fi os.FileInfo) string {
 
 // HandleTimeCache handles time-based caching for a HTTP request
 func HandleTimeCache(req *http.Request, w http.ResponseWriter, fi os.FileInfo) (handled bool) {
-	w.Header().Set("Cache-Control", GetCacheControl())
+	AddCacheControlToHeader(w.Header(), setting.StaticCacheTime)
 
 	ifModifiedSince := req.Header.Get("If-Modified-Since")
 	if ifModifiedSince != "" {
@@ -63,7 +68,7 @@ func HandleGenericETagCache(req *http.Request, w http.ResponseWriter, etag strin
 			return true
 		}
 	}
-	w.Header().Set("Cache-Control", GetCacheControl())
+	AddCacheControlToHeader(w.Header(), setting.StaticCacheTime)
 	return false
 }
 
diff --git a/modules/httpcache/httpcache_test.go b/modules/httpcache/httpcache_test.go
index fe5ca17956..68ac892c91 100644
--- a/modules/httpcache/httpcache_test.go
+++ b/modules/httpcache/httpcache_test.go
@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -24,6 +25,17 @@ func (m mockFileInfo) ModTime() time.Time { return time.Time{} }
 func (m mockFileInfo) IsDir() bool        { return false }
 func (m mockFileInfo) Sys() interface{}   { return nil }
 
+func countFormalHeaders(h http.Header) (c int) {
+	for k := range h {
+		// ignore our headers for internal usage
+		if strings.HasPrefix(k, "X-Gitea-") {
+			continue
+		}
+		c++
+	}
+	return c
+}
+
 func TestHandleFileETagCache(t *testing.T) {
 	fi := mockFileInfo{}
 	etag := `"MTBnaXRlYS50ZXN0TW9uLCAwMSBKYW4gMDAwMSAwMDowMDowMCBHTVQ="`
@@ -35,7 +47,7 @@ func TestHandleFileETagCache(t *testing.T) {
 		handled := HandleFileETagCache(req, w, fi)
 
 		assert.False(t, handled)
-		assert.Len(t, w.Header(), 2)
+		assert.Equal(t, 2, countFormalHeaders(w.Header()))
 		assert.Contains(t, w.Header(), "Cache-Control")
 		assert.Contains(t, w.Header(), "Etag")
 		assert.Equal(t, etag, w.Header().Get("Etag"))
@@ -49,7 +61,7 @@ func TestHandleFileETagCache(t *testing.T) {
 		handled := HandleFileETagCache(req, w, fi)
 
 		assert.False(t, handled)
-		assert.Len(t, w.Header(), 2)
+		assert.Equal(t, 2, countFormalHeaders(w.Header()))
 		assert.Contains(t, w.Header(), "Cache-Control")
 		assert.Contains(t, w.Header(), "Etag")
 		assert.Equal(t, etag, w.Header().Get("Etag"))
@@ -63,7 +75,7 @@ func TestHandleFileETagCache(t *testing.T) {
 		handled := HandleFileETagCache(req, w, fi)
 
 		assert.True(t, handled)
-		assert.Len(t, w.Header(), 1)
+		assert.Equal(t, 1, countFormalHeaders(w.Header()))
 		assert.Contains(t, w.Header(), "Etag")
 		assert.Equal(t, etag, w.Header().Get("Etag"))
 		assert.Equal(t, http.StatusNotModified, w.Code)
@@ -80,7 +92,7 @@ func TestHandleGenericETagCache(t *testing.T) {
 		handled := HandleGenericETagCache(req, w, etag)
 
 		assert.False(t, handled)
-		assert.Len(t, w.Header(), 2)
+		assert.Equal(t, 2, countFormalHeaders(w.Header()))
 		assert.Contains(t, w.Header(), "Cache-Control")
 		assert.Contains(t, w.Header(), "Etag")
 		assert.Equal(t, etag, w.Header().Get("Etag"))
@@ -94,7 +106,7 @@ func TestHandleGenericETagCache(t *testing.T) {
 		handled := HandleGenericETagCache(req, w, etag)
 
 		assert.False(t, handled)
-		assert.Len(t, w.Header(), 2)
+		assert.Equal(t, 2, countFormalHeaders(w.Header()))
 		assert.Contains(t, w.Header(), "Cache-Control")
 		assert.Contains(t, w.Header(), "Etag")
 		assert.Equal(t, etag, w.Header().Get("Etag"))
@@ -108,7 +120,7 @@ func TestHandleGenericETagCache(t *testing.T) {
 		handled := HandleGenericETagCache(req, w, etag)
 
 		assert.True(t, handled)
-		assert.Len(t, w.Header(), 1)
+		assert.Equal(t, 1, countFormalHeaders(w.Header()))
 		assert.Contains(t, w.Header(), "Etag")
 		assert.Equal(t, etag, w.Header().Get("Etag"))
 		assert.Equal(t, http.StatusNotModified, w.Code)
@@ -122,7 +134,7 @@ func TestHandleGenericETagCache(t *testing.T) {
 		handled := HandleGenericETagCache(req, w, etag)
 
 		assert.False(t, handled)
-		assert.Len(t, w.Header(), 2)
+		assert.Equal(t, 2, countFormalHeaders(w.Header()))
 		assert.Contains(t, w.Header(), "Cache-Control")
 		assert.Contains(t, w.Header(), "Etag")
 		assert.Equal(t, etag, w.Header().Get("Etag"))
@@ -136,7 +148,7 @@ func TestHandleGenericETagCache(t *testing.T) {
 		handled := HandleGenericETagCache(req, w, etag)
 
 		assert.True(t, handled)
-		assert.Len(t, w.Header(), 1)
+		assert.Equal(t, 1, countFormalHeaders(w.Header()))
 		assert.Contains(t, w.Header(), "Etag")
 		assert.Equal(t, etag, w.Header().Get("Etag"))
 		assert.Equal(t, http.StatusNotModified, w.Code)
diff --git a/modules/repository/commits.go b/modules/repository/commits.go
index 1558d85639..c86f0d570b 100644
--- a/modules/repository/commits.go
+++ b/modules/repository/commits.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/models/avatars"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	api "code.gitea.io/gitea/modules/structs"
@@ -139,14 +140,14 @@ func (pc *PushCommits) AvatarLink(email string) string {
 		return avatar
 	}
 
-	size := models.DefaultAvatarPixelSize * models.AvatarRenderedSizeFactor
+	size := avatars.DefaultAvatarPixelSize * avatars.AvatarRenderedSizeFactor
 
 	u, ok := pc.emailUsers[email]
 	if !ok {
 		var err error
 		u, err = models.GetUserByEmail(email)
 		if err != nil {
-			pc.avatars[email] = models.SizedAvatarLink(email, size)
+			pc.avatars[email] = avatars.GenerateEmailAvatarFastLink(email, size)
 			if !models.IsErrUserNotExist(err) {
 				log.Error("GetUserByEmail: %v", err)
 				return ""
@@ -156,7 +157,7 @@ func (pc *PushCommits) AvatarLink(email string) string {
 		}
 	}
 	if u != nil {
-		pc.avatars[email] = u.RealSizedAvatarLink(size)
+		pc.avatars[email] = u.AvatarLinkWithSize(size)
 	}
 
 	return pc.avatars[email]
diff --git a/modules/templates/helper.go b/modules/templates/helper.go
index 27d81ec9d9..b935eb6cc0 100644
--- a/modules/templates/helper.go
+++ b/modules/templates/helper.go
@@ -23,6 +23,7 @@ import (
 	"unicode"
 
 	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/models/avatars"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/emoji"
 	"code.gitea.io/gitea/modules/git"
@@ -550,16 +551,16 @@ func SVG(icon string, others ...interface{}) template.HTML {
 
 // Avatar renders user avatars. args: user, size (int), class (string)
 func Avatar(item interface{}, others ...interface{}) template.HTML {
-	size, class := parseOthers(models.DefaultAvatarPixelSize, "ui avatar image", others...)
+	size, class := parseOthers(avatars.DefaultAvatarPixelSize, "ui avatar image", others...)
 
 	if user, ok := item.(*models.User); ok {
-		src := user.RealSizedAvatarLink(size * models.AvatarRenderedSizeFactor)
+		src := user.AvatarLinkWithSize(size * avatars.AvatarRenderedSizeFactor)
 		if src != "" {
 			return AvatarHTML(src, size, class, user.DisplayName())
 		}
 	}
 	if user, ok := item.(*models.Collaborator); ok {
-		src := user.RealSizedAvatarLink(size * models.AvatarRenderedSizeFactor)
+		src := user.AvatarLinkWithSize(size * avatars.AvatarRenderedSizeFactor)
 		if src != "" {
 			return AvatarHTML(src, size, class, user.DisplayName())
 		}
@@ -575,7 +576,7 @@ func AvatarByAction(action *models.Action, others ...interface{}) template.HTML
 
 // RepoAvatar renders repo avatars. args: repo, size(int), class (string)
 func RepoAvatar(repo *models.Repository, others ...interface{}) template.HTML {
-	size, class := parseOthers(models.DefaultAvatarPixelSize, "ui avatar image", others...)
+	size, class := parseOthers(avatars.DefaultAvatarPixelSize, "ui avatar image", others...)
 
 	src := repo.RelAvatarLink()
 	if src != "" {
@@ -586,8 +587,8 @@ func RepoAvatar(repo *models.Repository, others ...interface{}) template.HTML {
 
 // AvatarByEmail renders avatars by email address. args: email, name, size (int), class (string)
 func AvatarByEmail(email string, name string, others ...interface{}) template.HTML {
-	size, class := parseOthers(models.DefaultAvatarPixelSize, "ui avatar image", others...)
-	src := models.SizedAvatarLink(email, size*models.AvatarRenderedSizeFactor)
+	size, class := parseOthers(avatars.DefaultAvatarPixelSize, "ui avatar image", others...)
+	src := avatars.GenerateEmailAvatarFastLink(email, size*avatars.AvatarRenderedSizeFactor)
 
 	if src != "" {
 		return AvatarHTML(src, size, class, name)