Direct avatar rendering (#13649)

* Direct avatar rendering

This adds new template helpers for avatar rendering which output image
elements with direct links to avatars which makes them cacheable by the
browsers.

This should be a major performance improvment for pages with many avatars.

* fix avatars of other user's profile pages

* fix top border on user avatar name

* uncircle avatars

* remove old incomplete avatar selector

* use title attribute for name and add it back on blame

* minor refactor

* tweak comments

* fix url path join and adjust test to new result

* dedupe functions

models/action.go

@@ -140,12 +140,6 @@ func (a *Action) GetDisplayNameTitle() string {
 	return a.GetActFullName()
 }
 
-// GetActAvatar the action's user's avatar link
-func (a *Action) GetActAvatar() string {
-	a.loadActUser()
-	return a.ActUser.RelAvatarLink()
-}
-
 // GetRepoUserName returns the name of the action repository owner.
 func (a *Action) GetRepoUserName() string {
 	a.loadRepo()

models/avatar.go

@@ -8,9 +8,13 @@ import (
 	"crypto/md5"
 	"fmt"
 	"net/url"
+	"path"
+	"strconv"
 	"strings"
 
+	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/cache"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 )
 
@@ -20,6 +24,28 @@ type EmailHash struct {
 	Email string `xorm:"UNIQUE NOT NULL"`
 }
 
+// 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, "/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
+
+// 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) {
@@ -32,8 +58,24 @@ func GetEmailForHash(md5Sum string) (string, error) {
 	})
 }
 
-// AvatarLink returns an avatar link for a provided email
-func AvatarLink(email string) string {
+// 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) string {
 	lowerEmail := strings.ToLower(strings.TrimSpace(email))
 	sum := fmt.Sprintf("%x", md5.Sum([]byte(lowerEmail)))
 	_, _ = cache.GetString("Avatar:"+sum, func() (string, error) {
@@ -57,3 +99,34 @@ func AvatarLink(email string) string {
 	})
 	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)
+	} else if !setting.DisableGravatar {
+		// copy GravatarSourceURL, because we will modify its Path.
+		copyOfGravatarSourceURL := *setting.GravatarSourceURL
+		avatarURL = &copyOfGravatarSourceURL
+		avatarURL.Path = path.Join(avatarURL.Path, HashEmail(email))
+	} else {
+		return DefaultAvatarLink()
+	}
+
+	return MakeFinalAvatarURL(avatarURL, size)
+}

models/avatar_test.go

@@ -0,0 +1,52 @@
+// 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 (
+	"net/url"
+	"testing"
+
+	"code.gitea.io/gitea/modules/setting"
+
+	"github.com/stretchr/testify/assert"
+)
+
+const gravatarSource = "https://secure.gravatar.com/avatar/"
+
+func disableGravatar() {
+	setting.EnableFederatedAvatar = false
+	setting.LibravatarService = nil
+	setting.DisableGravatar = true
+}
+
+func enableGravatar(t *testing.T) {
+	setting.DisableGravatar = false
+	var err error
+	setting.GravatarSourceURL, err = url.Parse(gravatarSource)
+	assert.NoError(t, err)
+}
+
+func TestHashEmail(t *testing.T) {
+	assert.Equal(t,
+		"d41d8cd98f00b204e9800998ecf8427e",
+		HashEmail(""),
+	)
+	assert.Equal(t,
+		"353cbad9b58e69c96154ad99f92bedc7",
+		HashEmail("gitea@example.com"),
+	)
+}
+
+func TestSizedAvatarLink(t *testing.T) {
+	disableGravatar()
+	assert.Equal(t, "/suburl/img/avatar_default.png",
+		SizedAvatarLink("gitea@example.com", 100))
+
+	enableGravatar(t)
+	assert.Equal(t,
+		"https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon&s=100",
+		SizedAvatarLink("gitea@example.com", 100),
+	)
+}

models/user_avatar.go

@@ -13,7 +13,6 @@ import (
 	"strings"
 
 	"code.gitea.io/gitea/modules/avatar"
-	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/storage"
@@ -41,7 +40,7 @@ func (u *User) generateRandomAvatar(e Engine) error {
 	}
 
 	if u.Avatar == "" {
-		u.Avatar = base.HashEmail(u.AvatarEmail)
+		u.Avatar = HashEmail(u.AvatarEmail)
 	}
 
 	if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error {
@@ -76,13 +75,13 @@ func (u *User) SizedRelAvatarLink(size int) string {
 //
 func (u *User) RealSizedAvatarLink(size int) string {
 	if u.ID == -1 {
-		return base.DefaultAvatarLink()
+		return DefaultAvatarLink()
 	}
 
 	switch {
 	case u.UseCustomAvatar:
 		if u.Avatar == "" {
-			return base.DefaultAvatarLink()
+			return DefaultAvatarLink()
 		}
 		return setting.AppSubURL + "/avatars/" + u.Avatar
 	case setting.DisableGravatar, setting.OfflineMode:
@@ -94,14 +93,14 @@ func (u *User) RealSizedAvatarLink(size int) string {
 
 		return setting.AppSubURL + "/avatars/" + u.Avatar
 	}
-	return base.SizedAvatarLink(u.AvatarEmail, size)
+	return SizedAvatarLink(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(base.DefaultAvatarSize)
+	return u.SizedRelAvatarLink(DefaultAvatarSize)
 }
 
 // AvatarLink returns user avatar absolute link.

modules/auth/sso/sspi_windows.go

@@ -168,7 +168,7 @@ func (s *SSPI) newUser(ctx *macaron.Context, username string, cfg *models.SSPICo
 		IsActive:                     cfg.AutoActivateUsers,
 		Language:                     cfg.DefaultLanguage,
 		UseCustomAvatar:              true,
-		Avatar:                       base.DefaultAvatarLink(),
+		Avatar:                       models.DefaultAvatarLink(),
 		EmailNotificationsPreference: models.EmailNotificationsDisabled,
 	}
 	if err := models.CreateUser(user); err != nil {

modules/base/tool.go

@@ -12,9 +12,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"net/http"
-	"net/url"
 	"os"
-	"path"
 	"path/filepath"
 	"runtime"
 	"strconv"
@@ -134,93 +132,6 @@ func CreateTimeLimitCode(data string, minutes int, startInf interface{}) string
 	return code
 }
 
-// HashEmail hashes email address to MD5 string.
-// https://en.gravatar.com/site/implement/hash/
-func HashEmail(email string) string {
-	return EncodeMD5(strings.ToLower(strings.TrimSpace(email)))
-}
-
-// DefaultAvatarLink the default avatar link
-func DefaultAvatarLink() string {
-	return setting.AppSubURL + "/img/avatar_default.png"
-}
-
-// DefaultAvatarSize is a sentinel value for the default avatar size, as
-// determined by the avatar-hosting service.
-const DefaultAvatarSize = -1
-
-// 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
-}
-
-// 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 {
-		var err error
-		avatarURL, err = libravatarURL(email)
-		if err != nil {
-			return DefaultAvatarLink()
-		}
-	} else if !setting.DisableGravatar {
-		// copy GravatarSourceURL, because we will modify its Path.
-		copyOfGravatarSourceURL := *setting.GravatarSourceURL
-		avatarURL = &copyOfGravatarSourceURL
-		avatarURL.Path = path.Join(avatarURL.Path, HashEmail(email))
-	} else {
-		return DefaultAvatarLink()
-	}
-
-	vals := avatarURL.Query()
-	vals.Set("d", "identicon")
-	if size != DefaultAvatarSize {
-		vals.Set("s", strconv.Itoa(size))
-	}
-	avatarURL.RawQuery = vals.Encode()
-	return avatarURL.String()
-}
-
-// SizedAvatarLinkWithDomain returns a sized link to the avatar for the given email
-// address.
-func SizedAvatarLinkWithDomain(email string, size int) string {
-	var avatarURL *url.URL
-	if setting.EnableFederatedAvatar && setting.LibravatarService != nil {
-		var err error
-		avatarURL, err = libravatarURL(email)
-		if err != nil {
-			return DefaultAvatarLink()
-		}
-	} else if !setting.DisableGravatar {
-		// copy GravatarSourceURL, because we will modify its Path.
-		copyOfGravatarSourceURL := *setting.GravatarSourceURL
-		avatarURL = &copyOfGravatarSourceURL
-		avatarURL.Path = path.Join(avatarURL.Path, HashEmail(email))
-	} else {
-		return DefaultAvatarLink()
-	}
-
-	vals := avatarURL.Query()
-	vals.Set("d", "identicon")
-	if size != DefaultAvatarSize {
-		vals.Set("s", strconv.Itoa(size))
-	}
-	avatarURL.RawQuery = vals.Encode()
-	return avatarURL.String()
-}
-
 // FileSize calculates the file size and generate user-friendly string.
 func FileSize(s int64) string {
 	return humanize.IBytes(uint64(s))

modules/base/tool_test.go

@@ -5,11 +5,8 @@
 package base
 
 import (
-	"net/url"
 	"testing"
 
-	"code.gitea.io/gitea/modules/setting"
-
 	"github.com/stretchr/testify/assert"
 )
 
@@ -56,44 +53,6 @@ func TestBasicAuthEncode(t *testing.T) {
 // TODO: Test VerifyTimeLimitCode()
 // TODO: Test CreateTimeLimitCode()
 
-func TestHashEmail(t *testing.T) {
-	assert.Equal(t,
-		"d41d8cd98f00b204e9800998ecf8427e",
-		HashEmail(""),
-	)
-	assert.Equal(t,
-		"353cbad9b58e69c96154ad99f92bedc7",
-		HashEmail("gitea@example.com"),
-	)
-}
-
-const gravatarSource = "https://secure.gravatar.com/avatar/"
-
-func disableGravatar() {
-	setting.EnableFederatedAvatar = false
-	setting.LibravatarService = nil
-	setting.DisableGravatar = true
-}
-
-func enableGravatar(t *testing.T) {
-	setting.DisableGravatar = false
-	var err error
-	setting.GravatarSourceURL, err = url.Parse(gravatarSource)
-	assert.NoError(t, err)
-}
-
-func TestSizedAvatarLink(t *testing.T) {
-	disableGravatar()
-	assert.Equal(t, "/img/avatar_default.png",
-		SizedAvatarLink("gitea@example.com", 100))
-
-	enableGravatar(t)
-	assert.Equal(t,
-		"https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon&s=100",
-		SizedAvatarLink("gitea@example.com", 100),
-	)
-}
-
 func TestFileSize(t *testing.T) {
 	var size int64 = 512
 	assert.Equal(t, "512 B", FileSize(size))

modules/repository/commits.go

@@ -123,7 +123,7 @@ func (pc *PushCommits) AvatarLink(email string) string {
 		var err error
 		u, err = models.GetUserByEmail(email)
 		if err != nil {
-			pc.avatars[email] = models.AvatarLink(email)
+			pc.avatars[email] = models.HashedAvatarLink(email)
 			if !models.IsErrUserNotExist(err) {
 				log.Error("GetUserByEmail: %v", err)
 				return ""