Refactor avatar package, support default avatar fallback

models/repo/avatar.go

@@ -41,20 +41,14 @@ func generateRandomAvatar(ctx context.Context, repo *Repository) error {
 	idToString := strconv.FormatInt(repo.ID, 10)
 
 	seed := idToString
-	img, err := avatar.RandomImage([]byte(seed))
-	if err != nil {
-		return fmt.Errorf("RandomImage: %w", err)
-	}
+	img := avatar.RandomImageDefaultSize([]byte(seed))
 
 	repo.Avatar = idToString
 
 	if err := storage.SaveFrom(storage.RepoAvatars, repo.CustomAvatarRelativePath(), func(w io.Writer) error {
-		if err := png.Encode(w, img); err != nil {
-			log.Error("Encode: %v", err)
-		}
-		return err
+		return png.Encode(w, img)
 	}); err != nil {
-		return fmt.Errorf("Failed to create dir %s: %w", repo.CustomAvatarRelativePath(), err)
+		return fmt.Errorf("failed to create dir %s: %w", repo.CustomAvatarRelativePath(), err)
 	}
 
 	log.Info("New random avatar created for repository: %d", repo.ID)

models/user/avatar.go

@@ -30,22 +30,16 @@ func GenerateRandomAvatar(ctx context.Context, u *User) error {
 		seed = u.Name
 	}
 
-	img, err := avatar.RandomImage([]byte(seed))
-	if err != nil {
-		return fmt.Errorf("RandomImage: %w", err)
-	}
+	img := avatar.RandomImageDefaultSize([]byte(seed))
 
 	u.Avatar = avatars.HashEmail(seed)
 
-	_, err = storage.Avatars.Stat(u.CustomAvatarRelativePath())
+	_, err := storage.Avatars.Stat(u.CustomAvatarRelativePath())
 	if err != nil {
 		// If unable to Stat the avatar file (usually it means non-existing), then try to save a new one
 		// 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 {
-			if err := png.Encode(w, img); err != nil {
-				log.Error("Encode: %v", err)
-			}
-			return nil
+			return png.Encode(w, img)
 		}); err != nil {
 			return fmt.Errorf("failed to save avatar %s: %w", u.CustomAvatarRelativePath(), err)
 		}

modules/assetfs/embed.go

@@ -260,7 +260,7 @@ func (fi *embeddedFileInfo) Mode() fs.FileMode {
 }
 
 func (fi *embeddedFileInfo) ModTime() time.Time {
-	return getExecutableModTime()
+	return GetExecutableModTime()
 }
 
 func (fi *embeddedFileInfo) IsDir() bool {
@@ -279,9 +279,9 @@ func (fi *embeddedFileInfo) Info() (fs.FileInfo, error) {
 	return fi, nil
 }
 
-// getExecutableModTime returns the modification time of the executable file.
+// GetExecutableModTime returns the modification time of the executable file.
 // In bindata, we can't use the ModTime of the files because we need to make the build reproducible
-var getExecutableModTime = sync.OnceValue(func() (modTime time.Time) {
+var GetExecutableModTime = sync.OnceValue(func() (modTime time.Time) {
 	exePath, err := os.Executable()
 	if err != nil {
 		return modTime

modules/avatar/avatar.go

@@ -28,21 +28,18 @@ import (
 // than the size after resizing.
 const DefaultAvatarSize = 256
 
-// RandomImageSize generates and returns a random avatar image unique to input data
+// RandomImageWithSize generates and returns a random avatar image unique to input data
 // in custom size (height and width).
-func RandomImageSize(size int, data []byte) (image.Image, error) {
+func RandomImageWithSize(size int, data []byte) image.Image {
 	// we use white as background, and use dark colors to draw blocks
-	imgMaker, err := identicon.New(size, color.White, identicon.DarkColors...)
-	if err != nil {
-		return nil, fmt.Errorf("identicon.New: %w", err)
-	}
-	return imgMaker.Make(data), nil
+	imgMaker := identicon.New(size, color.White, identicon.DarkColors)
+	return imgMaker.Make(data)
 }
 
-// RandomImage generates and returns a random avatar image unique to input data
+// RandomImageDefaultSize generates and returns a random avatar image unique to input data
 // in default size (height and width).
-func RandomImage(data []byte) (image.Image, error) {
-	return RandomImageSize(DefaultAvatarSize*setting.Avatar.RenderedSizeFactor, data)
+func RandomImageDefaultSize(data []byte) image.Image {
+	return RandomImageWithSize(DefaultAvatarSize*setting.Avatar.RenderedSizeFactor, data)
 }
 
 // processAvatarImage process the avatar image data, crop and resize it if necessary.

modules/avatar/avatar_test.go

@@ -15,19 +15,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func Test_RandomImageSize(t *testing.T) {
-	_, err := RandomImageSize(0, []byte("gitea@local"))
-	assert.Error(t, err)
-
-	_, err = RandomImageSize(64, []byte("gitea@local"))
-	assert.NoError(t, err)
-}
-
-func Test_RandomImage(t *testing.T) {
-	_, err := RandomImage([]byte("gitea@local"))
-	assert.NoError(t, err)
-}
-
 func Test_ProcessAvatarPNG(t *testing.T) {
 	setting.Avatar.MaxWidth = 4096
 	setting.Avatar.MaxHeight = 4096
@@ -134,3 +121,18 @@ func Test_ProcessAvatarImage(t *testing.T) {
 	_, err = processAvatarImage(origin, 262144)
 	assert.ErrorContains(t, err, "image width is too large: 10 > 5")
 }
+
+func BenchmarkRandomImage(b *testing.B) {
+	b.Run("size-48", func(b *testing.B) {
+		for b.Loop() {
+			// BenchmarkRandomImage/size-48-12         	   49549	     22899 ns/op
+			RandomImageWithSize(48, []byte("test-content"))
+		}
+	})
+	b.Run("size-96", func(b *testing.B) {
+		for b.Loop() {
+			// BenchmarkRandomImage/size-96-12         	   13816	     88187 ns/op
+			RandomImageWithSize(96, []byte("test-content"))
+		}
+	})
+}

modules/avatar/identicon/identicon.go

@@ -8,13 +8,14 @@ package identicon
 
 import (
 	"crypto/sha256"
-	"errors"
-	"fmt"
 	"image"
 	"image/color"
 )
 
-const minImageSize = 16
+const (
+	minImageSize = 16
+	maxImageSize = 2048
+)
 
 // Identicon is used to generate pseudo-random avatars
 type Identicon struct {
@@ -24,25 +25,17 @@ type Identicon struct {
 	rect       image.Rectangle
 }
 
-// New returns an Identicon struct with the correct settings
-// size image size
-// back background color
-// fore all possible foreground colors. only one foreground color will be picked randomly for one image
-func New(size int, back color.Color, fore ...color.Color) (*Identicon, error) {
-	if len(fore) == 0 {
-		return nil, errors.New("foreground is not set")
-	}
-
-	if size < minImageSize {
-		return nil, fmt.Errorf("size %d is smaller than min size %d", size, minImageSize)
-	}
-
+// New returns an Identicon struct.
+// Only one foreground color will be picked randomly for one image.
+func New(size int, backColor color.Color, foreColors []color.Color) *Identicon {
+	size = max(size, minImageSize)
+	size = min(size, maxImageSize)
 	return &Identicon{
-		foreColors: fore,
-		backColor:  back,
+		foreColors: foreColors,
+		backColor:  backColor,
 		size:       size,
 		rect:       image.Rect(0, 0, size, size),
-	}, nil
+	}
 }
 
 // Make generates an avatar by data

modules/avatar/identicon/identicon_test.go

@@ -23,7 +23,7 @@ func TestGenerate(t *testing.T) {
 	}
 
 	backColor := color.White
-	imgMaker, err := New(64, backColor, DarkColors...)
+	imgMaker, err := New(64, backColor, DarkColors)
 	assert.NoError(t, err)
 	for i := 0; i < 100; i++ {
 		s := strconv.Itoa(i)

modules/httpcache/httpcache.go

@@ -60,21 +60,6 @@ func CacheControlForPrivateStatic() *CacheControlOptions {
 	}
 }
 
-// HandleGenericETagCache handles ETag-based caching for a HTTP request.
-// It returns true if the request was handled.
-func HandleGenericETagCache(req *http.Request, w http.ResponseWriter, etag string) (handled bool) {
-	if len(etag) > 0 {
-		w.Header().Set("Etag", etag)
-		if checkIfNoneMatchIsValid(req, etag) {
-			w.WriteHeader(http.StatusNotModified)
-			return true
-		}
-	}
-	// not sure whether it is a public content, so just use "private" (old behavior)
-	SetCacheControlInHeader(w.Header(), CacheControlForPrivateStatic())
-	return false
-}
-
 // checkIfNoneMatchIsValid tests if the header If-None-Match matches the ETag
 func checkIfNoneMatchIsValid(req *http.Request, etag string) bool {
 	ifNoneMatch := req.Header.Get("If-None-Match")
@@ -89,18 +74,26 @@ func checkIfNoneMatchIsValid(req *http.Request, etag string) bool {
 	return false
 }
 
-// HandleGenericETagTimeCache handles ETag-based caching with Last-Modified caching for a HTTP request.
+func HandleGenericETagPublicCache(req *http.Request, w http.ResponseWriter, etag string, lastModified *time.Time) bool {
+	return handleGenericETagTimeCache(req, w, etag, lastModified, CacheControlForPublicStatic())
+}
+
+func HandleGenericETagPrivateCache(req *http.Request, w http.ResponseWriter, etag string, lastModified *time.Time) bool {
+	return handleGenericETagTimeCache(req, w, etag, lastModified, CacheControlForPrivateStatic())
+}
+
+// handleGenericETagTimeCache handles ETag-based caching with Last-Modified caching for the HTTP request.
 // It returns true if the request was handled.
-func HandleGenericETagTimeCache(req *http.Request, w http.ResponseWriter, etag string, lastModified *time.Time) (handled bool) {
-	if len(etag) > 0 {
+func handleGenericETagTimeCache(req *http.Request, w http.ResponseWriter, etag string, lastModified *time.Time, cacheControlOpts *CacheControlOptions) (handled bool) {
+	if etag != "" {
 		w.Header().Set("Etag", etag)
 	}
 	if lastModified != nil && !lastModified.IsZero() {
 		// http.TimeFormat required a UTC time, refer to https://pkg.go.dev/net/http#TimeFormat
 		w.Header().Set("Last-Modified", lastModified.UTC().Format(http.TimeFormat))
 	}
 
-	if len(etag) > 0 {
+	if etag != "" {
 		if checkIfNoneMatchIsValid(req, etag) {
 			w.WriteHeader(http.StatusNotModified)
 			return true
@@ -117,7 +110,6 @@ func HandleGenericETagTimeCache(req *http.Request, w http.ResponseWriter, etag s
 		}
 	}
 
-	// not sure whether it is a public content, so just use "private" (old behavior)
-	SetCacheControlInHeader(w.Header(), CacheControlForPrivateStatic())
+	SetCacheControlInHeader(w.Header(), cacheControlOpts)
 	return false
 }