From f0ba87fda88c7bb601eee17f3e3a72bea8a71521 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 6 Oct 2021 07:25:46 +0800 Subject: [PATCH] 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. --- integrations/user_avatar_test.go | 11 +- models/avatar.go | 148 -------------- models/avatars/avatar.go | 180 ++++++++++++++++++ models/{ => avatars}/avatar_test.go | 6 +- models/user_avatar.go | 65 +++---- modules/httpcache/httpcache.go | 19 +- modules/httpcache/httpcache_test.go | 28 ++- modules/repository/commits.go | 7 +- modules/templates/helper.go | 13 +- routers/web/repo/issue.go | 2 +- routers/web/user/avatar.go | 84 ++------ routers/web/web.go | 2 +- services/auth/sspi_windows.go | 3 +- templates/base/head.tmpl | 4 +- .../repo/issue/view_content/comments.tmpl | 2 +- 15 files changed, 274 insertions(+), 300 deletions(-) delete mode 100644 models/avatar.go create mode 100644 models/avatars/avatar.go rename models/{ => avatars}/avatar_test.go (89%) diff --git a/integrations/user_avatar_test.go b/integrations/user_avatar_test.go index c909fd6851..6de2914f7f 100644 --- a/integrations/user_avatar_test.go +++ b/integrations/user_avatar_test.go @@ -11,7 +11,6 @@ import ( "mime/multipart" "net/http" "net/url" - "strings" "testing" "code.gitea.io/gitea/models" @@ -75,14 +74,8 @@ func TestUserAvatar(t *testing.T) { user2 = db.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) // owner of the repo3, is an org req = NewRequest(t, "GET", user2.AvatarLink()) - resp := session.MakeRequest(t, req, http.StatusFound) - location := resp.Header().Get("Location") - if !strings.HasPrefix(location, "/avatars") { - assert.Fail(t, "Avatar location is not local: %s", location) - } - req = NewRequest(t, "GET", location) - session.MakeRequest(t, req, http.StatusOK) + _ = session.MakeRequest(t, req, http.StatusOK) - // Can't test if the response matches because the image is regened on upload but checking that this at least doesn't give a 404 should be enough. + // 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. }) } diff --git a/models/avatar.go b/models/avatar.go deleted file mode 100644 index 71b14ad915..0000000000 --- a/models/avatar.go +++ /dev/null @@ -1,148 +0,0 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package models - -import ( - "context" - "crypto/md5" - "fmt" - "net/url" - "path" - "strconv" - "strings" - - "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/modules/base" - "code.gitea.io/gitea/modules/cache" - "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" -) - -// EmailHash represents a pre-generated hash map -type EmailHash struct { - Hash string `xorm:"pk varchar(32)"` - Email string `xorm:"UNIQUE NOT NULL"` -} - -func init() { - db.RegisterModel(new(EmailHash)) -} - -// DefaultAvatarLink the default avatar link -func DefaultAvatarLink() string { - u, err := url.Parse(setting.AppSubURL) - if err != nil { - log.Error("GetUserByEmail: %v", err) - return "" - } - - u.Path = path.Join(u.Path, "/assets/img/avatar_default.png") - return u.String() -} - -// DefaultAvatarSize is a sentinel value for the default avatar size, as -// determined by the avatar-hosting service. -const DefaultAvatarSize = -1 - -// DefaultAvatarPixelSize is the default size in pixels of a rendered avatar -const DefaultAvatarPixelSize = 28 - -// AvatarRenderedSizeFactor is the factor by which the default size is increased for finer rendering -const AvatarRenderedSizeFactor = 4 - -// HashEmail hashes email address to MD5 string. -// https://en.gravatar.com/site/implement/hash/ -func HashEmail(email string) string { - return base.EncodeMD5(strings.ToLower(strings.TrimSpace(email))) -} - -// GetEmailForHash converts a provided md5sum to the email -func GetEmailForHash(md5Sum string) (string, error) { - return cache.GetString("Avatar:"+md5Sum, func() (string, error) { - emailHash := EmailHash{ - Hash: strings.ToLower(strings.TrimSpace(md5Sum)), - } - - _, err := db.GetEngine(db.DefaultContext).Get(&emailHash) - return emailHash.Email, err - }) -} - -// LibravatarURL returns the URL for the given email. This function should only -// be called if a federated avatar service is enabled. -func LibravatarURL(email string) (*url.URL, error) { - urlStr, err := setting.LibravatarService.FromEmail(email) - if err != nil { - log.Error("LibravatarService.FromEmail(email=%s): error %v", email, err) - return nil, err - } - u, err := url.Parse(urlStr) - if err != nil { - log.Error("Failed to parse libravatar url(%s): error %v", urlStr, err) - return nil, err - } - return u, nil -} - -// HashedAvatarLink returns an avatar link for a provided email -func HashedAvatarLink(email string, size int) string { - lowerEmail := strings.ToLower(strings.TrimSpace(email)) - sum := fmt.Sprintf("%x", md5.Sum([]byte(lowerEmail))) - _, _ = cache.GetString("Avatar:"+sum, func() (string, error) { - emailHash := &EmailHash{ - Email: lowerEmail, - Hash: sum, - } - // OK we're going to open a session just because I think that that might hide away any problems with postgres reporting errors - if err := db.WithTx(func(ctx context.Context) error { - has, err := db.GetEngine(ctx).Where("email = ? AND hash = ?", emailHash.Email, emailHash.Hash).Get(new(EmailHash)) - if has || err != nil { - // Seriously we don't care about any DB problems just return the lowerEmail - we expect the transaction to fail most of the time - return nil - } - _, _ = db.GetEngine(ctx).Insert(emailHash) - return nil - }); err != nil { - // Seriously we don't care about any DB problems just return the lowerEmail - we expect the transaction to fail most of the time - return lowerEmail, nil - } - return lowerEmail, nil - }) - if size > 0 { - return setting.AppSubURL + "/avatar/" + url.PathEscape(sum) + "?size=" + strconv.Itoa(size) - } - return setting.AppSubURL + "/avatar/" + url.PathEscape(sum) -} - -// MakeFinalAvatarURL constructs the final avatar URL string -func MakeFinalAvatarURL(u *url.URL, size int) string { - vals := u.Query() - vals.Set("d", "identicon") - if size != DefaultAvatarSize { - vals.Set("s", strconv.Itoa(size)) - } - u.RawQuery = vals.Encode() - return u.String() -} - -// SizedAvatarLink returns a sized link to the avatar for the given email address. -func SizedAvatarLink(email string, size int) string { - var avatarURL *url.URL - if setting.EnableFederatedAvatar && setting.LibravatarService != nil { - // This is the slow path that would need to call LibravatarURL() which - // does DNS lookups. Avoid it by issuing a redirect so we don't block - // the template render with network requests. - return HashedAvatarLink(email, size) - } else if !setting.DisableGravatar { - // copy GravatarSourceURL, because we will modify its Path. - copyOfGravatarSourceURL := *setting.GravatarSourceURL - avatarURL = ©OfGravatarSourceURL - avatarURL.Path = path.Join(avatarURL.Path, HashEmail(email)) - } else { - return DefaultAvatarLink() - } - - return MakeFinalAvatarURL(avatarURL, size) -} diff --git a/models/avatars/avatar.go b/models/avatars/avatar.go new file mode 100644 index 0000000000..0a1445d2f2 --- /dev/null +++ b/models/avatars/avatar.go @@ -0,0 +1,180 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package avatars + +import ( + "context" + "net/url" + "path" + "strconv" + "strings" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" +) + +// DefaultAvatarPixelSize is the default size in pixels of a rendered avatar +const DefaultAvatarPixelSize = 28 + +// AvatarRenderedSizeFactor is the factor by which the default size is increased for finer rendering +const AvatarRenderedSizeFactor = 4 + +// EmailHash represents a pre-generated hash map (mainly used by LibravatarURL, it queries email server's DNS records) +type EmailHash struct { + Hash string `xorm:"pk varchar(32)"` + Email string `xorm:"UNIQUE NOT NULL"` +} + +func init() { + db.RegisterModel(new(EmailHash)) +} + +// DefaultAvatarLink the default avatar link +func DefaultAvatarLink() string { + u, err := url.Parse(setting.AppSubURL) + if err != nil { + log.Error("GetUserByEmail: %v", err) + return "" + } + + u.Path = path.Join(u.Path, "/assets/img/avatar_default.png") + return u.String() +} + +// HashEmail hashes email address to MD5 string. https://en.gravatar.com/site/implement/hash/ +func HashEmail(email string) string { + return base.EncodeMD5(strings.ToLower(strings.TrimSpace(email))) +} + +// GetEmailForHash converts a provided md5sum to the email +func GetEmailForHash(md5Sum string) (string, error) { + return cache.GetString("Avatar:"+md5Sum, func() (string, error) { + emailHash := EmailHash{ + Hash: strings.ToLower(strings.TrimSpace(md5Sum)), + } + + _, err := db.GetEngine(db.DefaultContext).Get(&emailHash) + return emailHash.Email, err + }) +} + +// LibravatarURL returns the URL for the given email. Slow due to the DNS lookup. +// This function should only be called if a federated avatar service is enabled. +func LibravatarURL(email string) (*url.URL, error) { + urlStr, err := setting.LibravatarService.FromEmail(email) + if err != nil { + log.Error("LibravatarService.FromEmail(email=%s): error %v", email, err) + return nil, err + } + u, err := url.Parse(urlStr) + if err != nil { + log.Error("Failed to parse libravatar url(%s): error %v", urlStr, err) + return nil, err + } + return u, nil +} + +// saveEmailHash returns an avatar link for a provided email, +// the email and hash are saved into database, which will be used by GetEmailForHash later +func saveEmailHash(email string) string { + lowerEmail := strings.ToLower(strings.TrimSpace(email)) + emailHash := HashEmail(lowerEmail) + _, _ = cache.GetString("Avatar:"+emailHash, func() (string, error) { + emailHash := &EmailHash{ + Email: lowerEmail, + Hash: emailHash, + } + // OK we're going to open a session just because I think that that might hide away any problems with postgres reporting errors + if err := db.WithTx(func(ctx context.Context) error { + has, err := db.GetEngine(ctx).Where("email = ? AND hash = ?", emailHash.Email, emailHash.Hash).Get(new(EmailHash)) + if has || err != nil { + // Seriously we don't care about any DB problems just return the lowerEmail - we expect the transaction to fail most of the time + return nil + } + _, _ = db.GetEngine(ctx).Insert(emailHash) + return nil + }); err != nil { + // Seriously we don't care about any DB problems just return the lowerEmail - we expect the transaction to fail most of the time + return lowerEmail, nil + } + return lowerEmail, nil + }) + return emailHash +} + +// GenerateUserAvatarFastLink returns a fast link (302) to the user's avatar: "/user/avatar/${User.Name}/${size}" +func GenerateUserAvatarFastLink(userName string, size int) string { + if size < 0 { + size = 0 + } + return setting.AppSubURL + "/user/avatar/" + userName + "/" + strconv.Itoa(size) +} + +// GenerateUserAvatarImageLink returns a link for `User.Avatar` image file: "/avatars/${User.Avatar}" +func GenerateUserAvatarImageLink(userAvatar string, size int) string { + if size > 0 { + return setting.AppSubURL + "/avatars/" + userAvatar + "?size=" + strconv.Itoa(size) + } + return setting.AppSubURL + "/avatars/" + userAvatar +} + +// 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() + urlQuery.Set("d", "identicon") + if size > 0 { + urlQuery.Set("s", strconv.Itoa(size)) + } + u.RawQuery = urlQuery.Encode() + return u.String() +} + +// generateEmailAvatarLink returns a email avatar link. +// if final is true, it may use a slow path (eg: query DNS). +// if final is false, it always uses a fast path. +func generateEmailAvatarLink(email string, size int, final bool) string { + email = strings.TrimSpace(email) + if email == "" { + return DefaultAvatarLink() + } + + var err error + if setting.EnableFederatedAvatar && setting.LibravatarService != nil { + emailHash := saveEmailHash(email) + if final { + // for final link, we can spend more time on slow external query + var avatarURL *url.URL + if avatarURL, err = LibravatarURL(email); err != nil { + return DefaultAvatarLink() + } + return generateRecognizedAvatarURL(*avatarURL, size) + } + // for non-final link, we should return fast (use a 302 redirection link) + urlStr := setting.AppSubURL + "/avatar/" + emailHash + if size > 0 { + urlStr += "?size=" + strconv.Itoa(size) + } + return urlStr + } else if !setting.DisableGravatar { + // copy GravatarSourceURL, because we will modify its Path. + avatarURLCopy := *setting.GravatarSourceURL + avatarURLCopy.Path = path.Join(avatarURLCopy.Path, HashEmail(email)) + return generateRecognizedAvatarURL(avatarURLCopy, size) + } + return DefaultAvatarLink() +} + +//GenerateEmailAvatarFastLink returns a avatar link (fast, the link may be a delegated one: "/avatar/${hash}") +func GenerateEmailAvatarFastLink(email string, size int) string { + return generateEmailAvatarLink(email, size, false) +} + +//GenerateEmailAvatarFinalLink returns a avatar final link (maybe slow) +func GenerateEmailAvatarFinalLink(email string, size int) string { + return generateEmailAvatarLink(email, size, true) +} diff --git a/models/avatar_test.go b/models/avatars/avatar_test.go similarity index 89% rename from models/avatar_test.go rename to models/avatars/avatar_test.go index 09f1a8066d..4d6255ca5f 100644 --- a/models/avatar_test.go +++ b/models/avatars/avatar_test.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a MIT-style // license that can be found in the LICENSE file. -package models +package avatars import ( "net/url" @@ -44,11 +44,11 @@ func TestSizedAvatarLink(t *testing.T) { disableGravatar() assert.Equal(t, "/testsuburl/assets/img/avatar_default.png", - SizedAvatarLink("gitea@example.com", 100)) + GenerateEmailAvatarFastLink("gitea@example.com", 100)) enableGravatar(t) assert.Equal(t, "https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon&s=100", - SizedAvatarLink("gitea@example.com", 100), + GenerateEmailAvatarFastLink("gitea@example.com", 100), ) } diff --git a/models/user_avatar.go b/models/user_avatar.go index a7ca1c9151..b8296568c2 100644 --- a/models/user_avatar.go +++ b/models/user_avatar.go @@ -9,9 +9,8 @@ import ( "fmt" "image/png" "io" - "strconv" - "strings" + "code.gitea.io/gitea/models/avatars" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/avatar" "code.gitea.io/gitea/modules/log" @@ -40,7 +39,7 @@ func (u *User) generateRandomAvatar(e db.Engine) error { return fmt.Errorf("RandomImage: %v", err) } - u.Avatar = HashEmail(seed) + u.Avatar = avatars.HashEmail(seed) // Don't share the images so that we can delete them easily if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error { @@ -60,61 +59,41 @@ func (u *User) generateRandomAvatar(e db.Engine) error { return nil } -// SizedRelAvatarLink returns a link to the user's avatar via -// the local explore page. Function returns immediately. -// When applicable, the link is for an avatar of the indicated size (in pixels). -func (u *User) SizedRelAvatarLink(size int) string { - return setting.AppSubURL + "/user/avatar/" + u.Name + "/" + strconv.Itoa(size) -} - -// RealSizedAvatarLink returns a link to the user's avatar. When -// applicable, the link is for an avatar of the indicated size (in pixels). -// -// This function make take time to return when federated avatars -// are in use, due to a DNS lookup need -// -func (u *User) RealSizedAvatarLink(size int) string { +// AvatarLinkWithSize returns a link to the user's avatar with size. size <= 0 means default size +func (u *User) AvatarLinkWithSize(size int) string { if u.ID == -1 { - return DefaultAvatarLink() + // ghost user + return avatars.DefaultAvatarLink() } + useLocalAvatar := false + autoGenerateAvatar := false + switch { case u.UseCustomAvatar: - if u.Avatar == "" { - return DefaultAvatarLink() - } - if size > 0 { - return setting.AppSubURL + "/avatars/" + u.Avatar + "?size=" + strconv.Itoa(size) - } - return setting.AppSubURL + "/avatars/" + u.Avatar + useLocalAvatar = true case setting.DisableGravatar, setting.OfflineMode: - if u.Avatar == "" { + useLocalAvatar = true + autoGenerateAvatar = true + } + + if useLocalAvatar { + if u.Avatar == "" && autoGenerateAvatar { if err := u.GenerateRandomAvatar(); err != nil { log.Error("GenerateRandomAvatar: %v", err) } } - if size > 0 { - return setting.AppSubURL + "/avatars/" + u.Avatar + "?size=" + strconv.Itoa(size) + if u.Avatar == "" { + return avatars.DefaultAvatarLink() } - return setting.AppSubURL + "/avatars/" + u.Avatar + return avatars.GenerateUserAvatarImageLink(u.Avatar, size) } - return SizedAvatarLink(u.AvatarEmail, size) + return avatars.GenerateEmailAvatarFastLink(u.AvatarEmail, size) } -// RelAvatarLink returns a relative link to the user's avatar. The link -// may either be a sub-URL to this site, or a full URL to an external avatar -// service. -func (u *User) RelAvatarLink() string { - return u.SizedRelAvatarLink(DefaultAvatarSize) -} - -// AvatarLink returns user avatar absolute link. +// AvatarLink returns a avatar link with default size func (u *User) AvatarLink() string { - link := u.RelAvatarLink() - if link[0] == '/' && link[1] != '/' { - return setting.AppURL + strings.TrimPrefix(link, setting.AppSubURL)[1:] - } - return link + return u.AvatarLinkWithSize(0) } // UploadAvatar saves custom avatar for user. 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) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 03bdbec80c..64c4ae41f4 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -2614,5 +2614,5 @@ func handleTeamMentions(ctx *context.Context) { ctx.Data["MentionableTeams"] = ctx.Repo.Owner.Teams ctx.Data["MentionableTeamsOrg"] = ctx.Repo.Owner.Name - ctx.Data["MentionableTeamsOrgAvatar"] = ctx.Repo.Owner.RelAvatarLink() + ctx.Data["MentionableTeamsOrgAvatar"] = ctx.Repo.Owner.AvatarLink() } diff --git a/routers/web/user/avatar.go b/routers/web/user/avatar.go index 2df5c148f7..f39bcc36d3 100644 --- a/routers/web/user/avatar.go +++ b/routers/web/user/avatar.go @@ -5,100 +5,50 @@ package user import ( - "errors" - "net/url" - "path" - "strconv" "strings" + "time" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/avatars" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/httpcache" - "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" ) func cacheableRedirect(ctx *context.Context, location string) { - ctx.Resp.Header().Set("Cache-Control", httpcache.GetCacheControl()) + // here we should not use `setting.StaticCacheTime`, it is pretty long (default: 6 hours) + // we must make sure the redirection cache time is short enough, otherwise a user won't see the updated avatar in 6 hours + // it's OK to make the cache time short, it is only a redirection, and doesn't cost much to make a new request + httpcache.AddCacheControlToHeader(ctx.Resp.Header(), 5*time.Minute) ctx.Redirect(location) } -// Avatar redirect browser to user avatar of requested size -func Avatar(ctx *context.Context) { +// AvatarByUserName redirect browser to user avatar of requested size +func AvatarByUserName(ctx *context.Context) { userName := ctx.Params(":username") - size, err := strconv.Atoi(ctx.Params(":size")) - if err != nil { - ctx.ServerError("Invalid avatar size", err) - return - } - - log.Debug("Asked avatar for user %v and size %v", userName, size) + size := int(ctx.ParamsInt64(":size")) var user *models.User if strings.ToLower(userName) != "ghost" { - user, err = models.GetUserByName(userName) - if err != nil { - if models.IsErrUserNotExist(err) { - ctx.ServerError("Requested avatar for invalid user", err) - } else { - ctx.ServerError("Retrieving user by name", err) - } + var err error + if user, err = models.GetUserByName(userName); err != nil { + ctx.ServerError("Invalid user: "+userName, err) return } } else { user = models.NewGhostUser() } - cacheableRedirect(ctx, user.RealSizedAvatarLink(size)) + cacheableRedirect(ctx, user.AvatarLinkWithSize(size)) } -// AvatarByEmailHash redirects the browser to the appropriate Avatar link +// AvatarByEmailHash redirects the browser to the email avatar link func AvatarByEmailHash(ctx *context.Context) { - var err error - hash := ctx.Params(":hash") - if len(hash) == 0 { - ctx.ServerError("invalid avatar hash", errors.New("hash cannot be empty")) - return - } - - var email string - email, err = models.GetEmailForHash(hash) + email, err := avatars.GetEmailForHash(hash) if err != nil { - ctx.ServerError("invalid avatar hash", err) - return - } - if len(email) == 0 { - cacheableRedirect(ctx, models.DefaultAvatarLink()) + ctx.ServerError("invalid avatar hash: "+hash, err) return } size := ctx.FormInt("size") - if size == 0 { - size = models.DefaultAvatarSize - } - - var avatarURL *url.URL - - if setting.EnableFederatedAvatar && setting.LibravatarService != nil { - avatarURL, err = models.LibravatarURL(email) - if err != nil { - avatarURL, err = url.Parse(models.DefaultAvatarLink()) - if err != nil { - ctx.ServerError("invalid default avatar url", err) - return - } - } - } else if !setting.DisableGravatar { - copyOfGravatarSourceURL := *setting.GravatarSourceURL - avatarURL = ©OfGravatarSourceURL - avatarURL.Path = path.Join(avatarURL.Path, hash) - } else { - avatarURL, err = url.Parse(models.DefaultAvatarLink()) - if err != nil { - ctx.ServerError("invalid default avatar url", err) - return - } - } - - cacheableRedirect(ctx, models.MakeFinalAvatarURL(avatarURL, size)) + cacheableRedirect(ctx, avatars.GenerateEmailAvatarFinalLink(email, size)) } diff --git a/routers/web/web.go b/routers/web/web.go index a56a2250bd..e12552f4d4 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -366,7 +366,7 @@ func RegisterRoutes(m *web.Route) { m.Get("/activate", user.Activate, reqSignIn) m.Post("/activate", user.ActivatePost, reqSignIn) m.Any("/activate_email", user.ActivateEmail) - m.Get("/avatar/{username}/{size}", user.Avatar) + m.Get("/avatar/{username}/{size}", user.AvatarByUserName) m.Get("/email2user", user.Email2User) m.Get("/recover_account", user.ResetPasswd) m.Post("/recover_account", user.ResetPasswdPost) diff --git a/services/auth/sspi_windows.go b/services/auth/sspi_windows.go index a4c39b0064..346e943988 100644 --- a/services/auth/sspi_windows.go +++ b/services/auth/sspi_windows.go @@ -10,6 +10,7 @@ import ( "strings" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/avatars" "code.gitea.io/gitea/models/login" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" @@ -193,7 +194,7 @@ func (s *SSPI) newUser(username string, cfg *sspi.Source) (*models.User, error) IsActive: cfg.AutoActivateUsers, Language: cfg.DefaultLanguage, UseCustomAvatar: true, - Avatar: models.DefaultAvatarLink(), + Avatar: avatars.DefaultAvatarLink(), EmailNotificationsPreference: models.EmailNotificationsDisabled, } if err := models.CreateUser(user); err != nil { diff --git a/templates/base/head.tmpl b/templates/base/head.tmpl index 328661eb9d..15f2826abf 100644 --- a/templates/base/head.tmpl +++ b/templates/base/head.tmpl @@ -48,11 +48,11 @@ tributeValues: Array.from(new Map([ {{ range .Participants }} ['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}', - name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.RelAvatarLink}}'}], + name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink}}'}], {{ end }} {{ range .Assignees }} ['{{.Name}}', {key: '{{.Name}} {{.FullName}}', value: '{{.Name}}', - name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.RelAvatarLink}}'}], + name: '{{.Name}}', fullname: '{{.FullName}}', avatar: '{{.AvatarLink}}'}], {{ end }} {{ range .MentionableTeams }} ['{{$.MentionableTeamsOrg}}/{{.Name}}', {key: '{{$.MentionableTeamsOrg}}/{{.Name}}', value: '{{$.MentionableTeamsOrg}}/{{.Name}}', diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index dabba7bc74..c7d7574231 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -746,7 +746,7 @@
- + {{svg "octicon-x" 16}}