fix(avatar): recover gracefully from broken avatar cache (#1379)

pkg/modules/avatar/initials/initials.go

@@ -18,6 +18,7 @@ package initials
 
 import (
 	"bytes"
+	"fmt"
 	"image"
 	"image/color"
 	"image/draw"
@@ -133,6 +134,15 @@ func getCacheKey(prefix string, keys ...int64) string {
 }
 
 func getAvatarForUser(u *user.User) (fullSizeAvatar *image.RGBA64, err error) {
+	return getAvatarForUserWithDepth(u, 0)
+}
+
+func getAvatarForUserWithDepth(u *user.User, recursionDepth int) (fullSizeAvatar *image.RGBA64, err error) {
+	// Prevent infinite recursion - max 3 attempts
+	if recursionDepth >= 3 {
+		return nil, fmt.Errorf("maximum recursion depth reached while generating avatar for user %d", u.ID)
+	}
+
 	cacheKey := getCacheKey("full", u.ID)
 
 	result, err := keyvalue.Remember(cacheKey, func() (any, error) {
@@ -155,7 +165,20 @@ func getAvatarForUser(u *user.User) (fullSizeAvatar *image.RGBA64, err error) {
 		return nil, err
 	}
 
-	aa := result.(image.RGBA64)
+	// Safe type assertion to handle cases where cached data might be corrupted or in legacy format
+	aa, ok := result.(image.RGBA64)
+	if !ok {
+		// Log the type mismatch with the actual stored value for debugging
+		log.Errorf("Invalid cached image type for user %d. Expected image.RGBA64, got %T with value: %+v. Clearing cache and regenerating.", u.ID, result, result)
+
+		// Clear the invalid cache entry
+		if err := keyvalue.Del(cacheKey); err != nil {
+			log.Errorf("Failed to clear invalid cache entry for key %s: %v", cacheKey, err)
+		}
+
+		// Regenerate the avatar by calling the function again (without the corrupted cache)
+		return getAvatarForUserWithDepth(u, recursionDepth+1)
+	}
 
 	return &aa, nil
 }
@@ -168,6 +191,15 @@ type CachedAvatar struct {
 
 // GetAvatar returns an initials avatar for a user
 func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType string, err error) {
+	return p.getAvatarWithDepth(u, size, 0)
+}
+
+func (p *Provider) getAvatarWithDepth(u *user.User, size int64, recursionDepth int) (avatar []byte, mimeType string, err error) {
+	// Prevent infinite recursion - max 3 attempts
+	if recursionDepth >= 3 {
+		return nil, "", fmt.Errorf("maximum recursion depth reached while generating avatar for user %d, size %d", u.ID, size)
+	}
+
 	cacheKey := getCacheKey("resized", u.ID, size)
 
 	result, err := keyvalue.Remember(cacheKey, func() (any, error) {
@@ -197,6 +229,20 @@ func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType
 		return nil, "", err
 	}
 
-	cachedAvatar := result.(CachedAvatar)
+	// Safe type assertion to handle cases where cached data might be corrupted or in legacy format
+	cachedAvatar, ok := result.(CachedAvatar)
+	if !ok {
+		// Log the type mismatch with the actual stored value for debugging
+		log.Errorf("Invalid cached avatar type for user %d, size %d. Expected CachedAvatar, got %T with value: %+v. Clearing cache and regenerating.", u.ID, size, result, result)
+
+		// Clear the invalid cache entry
+		if err := keyvalue.Del(cacheKey); err != nil {
+			log.Errorf("Failed to clear invalid cache entry for key %s: %v", cacheKey, err)
+		}
+
+		// Regenerate the avatar by calling the function again (without the corrupted cache)
+		return p.getAvatarWithDepth(u, size, recursionDepth+1)
+	}
+
 	return cachedAvatar.Content, cachedAvatar.MimeType, nil
 }

pkg/modules/avatar/initials/initials_test.go

@@ -0,0 +1,179 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-present Vikunja and contributors. All rights reserved.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package initials
+
+import (
+	"image"
+	"os"
+	"testing"
+
+	"code.vikunja.io/api/pkg/log"
+	"code.vikunja.io/api/pkg/modules/keyvalue"
+	"code.vikunja.io/api/pkg/user"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestMain initializes the test environment
+func TestMain(m *testing.M) {
+	// Initialize logger for tests
+	log.InitLogger()
+
+	os.Exit(m.Run())
+}
+
+func TestGetAvatar(t *testing.T) {
+	// Initialize storage for testing
+	keyvalue.InitStorage()
+
+	t.Run("handles invalid cached type", func(t *testing.T) {
+		provider := &Provider{}
+
+		// Create a test user
+		testUser := &user.User{
+			ID:       999999, // Use a high ID to avoid conflicts
+			Name:     "Test User",
+			Username: "testuser",
+		}
+
+		// Simulate corrupted cached data by storing a string instead of CachedAvatar
+		cacheKey := getCacheKey("resized", testUser.ID, 64)
+		err := keyvalue.Put(cacheKey, "corrupted_string_data")
+		require.NoError(t, err)
+
+		// This should not panic but should handle the type assertion gracefully
+		// and regenerate the avatar
+		avatar, mimeType, err := provider.GetAvatar(testUser, 64)
+
+		// The function should handle the type assertion failure gracefully
+		// and regenerate the avatar successfully
+		require.NoError(t, err)
+		assert.NotNil(t, avatar)
+		assert.Equal(t, "image/png", mimeType)
+		assert.NotEmpty(t, avatar, "Avatar should contain image data")
+	})
+
+	t.Run("handles valid cached type", func(t *testing.T) {
+		provider := &Provider{}
+
+		// Create a test user
+		testUser := &user.User{
+			ID:       888888, // Use a different ID to avoid cache conflicts
+			Name:     "Valid User",
+			Username: "validuser",
+		}
+
+		// Store a valid cached avatar
+		cacheKey := getCacheKey("resized", testUser.ID, 32)
+		validCachedAvatar := CachedAvatar{
+			Content:  []byte("fake_image_data"),
+			MimeType: "image/png",
+		}
+		err := keyvalue.Put(cacheKey, validCachedAvatar)
+		require.NoError(t, err)
+
+		// This should work correctly with the valid cached data
+		avatar, mimeType, err := provider.GetAvatar(testUser, 32)
+
+		// Should return the cached data successfully
+		require.NoError(t, err)
+		assert.Equal(t, []byte("fake_image_data"), avatar)
+		assert.Equal(t, "image/png", mimeType)
+	})
+
+	t.Run("generates valid initials", func(t *testing.T) {
+		provider := &Provider{}
+
+		// Test with name
+		testUser1 := &user.User{
+			ID:       555555,
+			Name:     "John Doe",
+			Username: "johndoe",
+		}
+
+		avatar1, mimeType1, err1 := provider.GetAvatar(testUser1, 128)
+		require.NoError(t, err1)
+		assert.NotNil(t, avatar1)
+		assert.Equal(t, "image/png", mimeType1)
+		assert.NotEmpty(t, avatar1)
+
+		// Test with username when name is empty
+		testUser2 := &user.User{
+			ID:       444444,
+			Name:     "", // Empty name
+			Username: "jane_smith",
+		}
+
+		avatar2, mimeType2, err2 := provider.GetAvatar(testUser2, 128)
+		require.NoError(t, err2)
+		assert.NotNil(t, avatar2)
+		assert.Equal(t, "image/png", mimeType2)
+		assert.NotEmpty(t, avatar2)
+	})
+}
+
+func TestGetAvatarForUser(t *testing.T) {
+	// Initialize storage for testing
+	keyvalue.InitStorage()
+
+	t.Run("handles invalid cached type", func(t *testing.T) {
+		// Create a test user
+		testUser := &user.User{
+			ID:       777777, // Use another unique ID
+			Name:     "Full Size Test User",
+			Username: "fullsizeuser",
+		}
+
+		// Simulate corrupted cached data by storing a string instead of image.RGBA64
+		cacheKey := getCacheKey("full", testUser.ID)
+		err := keyvalue.Put(cacheKey, "corrupted_image_data")
+		require.NoError(t, err)
+
+		// This should not panic but should handle the type assertion gracefully
+		// and regenerate the full size avatar
+		fullAvatar, err := getAvatarForUser(testUser)
+
+		// The function should handle the type assertion failure gracefully
+		// and generate a new avatar successfully
+		require.NoError(t, err)
+		assert.NotNil(t, fullAvatar)
+		assert.IsType(t, &image.RGBA64{}, fullAvatar)
+	})
+
+	t.Run("handles valid cached type", func(t *testing.T) {
+		// Create a test user
+		testUser := &user.User{
+			ID:       666666, // Use another unique ID
+			Name:     "Valid Full Size User",
+			Username: "validfulluser",
+		}
+
+		// Create a valid image.RGBA64 for caching
+		validImage := image.NewRGBA64(image.Rect(0, 0, 64, 64))
+		cacheKey := getCacheKey("full", testUser.ID)
+		err := keyvalue.Put(cacheKey, *validImage)
+		require.NoError(t, err)
+
+		// This should work correctly with the valid cached data
+		fullAvatar, err := getAvatarForUser(testUser)
+
+		// Should return the cached image successfully
+		require.NoError(t, err)
+		assert.NotNil(t, fullAvatar)
+		assert.IsType(t, &image.RGBA64{}, fullAvatar)
+	})
+}

pkg/modules/avatar/upload/upload.go

@@ -18,6 +18,7 @@ package upload
 
 import (
 	"bytes"
+	"fmt"
 	"image"
 	"image/png"
 	"io"
@@ -51,11 +52,25 @@ type CachedAvatar struct {
 
 // GetAvatar returns an uploaded user avatar
 func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType string, err error) {
+	return p.getAvatarWithDepth(u, size, 0)
+}
+
+func (p *Provider) getAvatarWithDepth(u *user.User, size int64, recursionDepth int) (avatar []byte, mimeType string, err error) {
+	// Prevent infinite recursion - max 3 attempts
+	if recursionDepth >= 3 {
+		return nil, "", fmt.Errorf("maximum recursion depth reached while generating avatar for user %d, size %d", u.ID, size)
+	}
+
 	cacheKey := CacheKeyPrefix + strconv.Itoa(int(u.ID)) + "_" + strconv.FormatInt(size, 10)
 
 	result, err := keyvalue.Remember(cacheKey, func() (any, error) {
 		log.Debugf("Uploaded avatar for user %d and size %d not cached, resizing and caching.", u.ID, size)
 
+		// Check if user has an avatar file ID
+		if u.AvatarFileID == 0 {
+			return nil, fmt.Errorf("user %d has no avatar file", u.ID)
+		}
+
 		// If we get this far, the avatar is either not cached at all or not in this size
 		f := &files.File{ID: u.AvatarFileID}
 		if err := f.LoadFileByID(); err != nil {
@@ -92,7 +107,20 @@ func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType
 		return nil, "", err
 	}
 
-	cachedAvatar := result.(CachedAvatar)
+	// Safe type assertion to handle cases where cached data might be corrupted or in legacy format
+	cachedAvatar, ok := result.(CachedAvatar)
+	if !ok {
+		// Log the type mismatch with the actual stored value for debugging
+		log.Errorf("Invalid cached avatar type for user %d, size %d. Expected CachedAvatar, got %T with value: %+v. Clearing cache and regenerating.", u.ID, size, result, result)
+
+		// Clear the invalid cache entry
+		if err := keyvalue.Del(cacheKey); err != nil {
+			log.Errorf("Failed to clear invalid cache entry for key %s: %v", cacheKey, err)
+		}
+
+		// Regenerate the avatar by calling the function again (without the corrupted cache)
+		return p.getAvatarWithDepth(u, size, recursionDepth+1)
+	}
 
 	return cachedAvatar.Content, cachedAvatar.MimeType, nil
 }

pkg/modules/avatar/upload/upload_test.go

@@ -0,0 +1,93 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-present Vikunja and contributors. All rights reserved.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package upload
+
+import (
+	"os"
+	"testing"
+
+	"code.vikunja.io/api/pkg/log"
+	"code.vikunja.io/api/pkg/modules/keyvalue"
+	"code.vikunja.io/api/pkg/user"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestMain initializes the test environment
+func TestMain(m *testing.M) {
+	// Initialize logger for tests
+	log.InitLogger()
+
+	os.Exit(m.Run())
+}
+
+func TestGetAvatar(t *testing.T) {
+	// Initialize storage for testing
+	keyvalue.InitStorage()
+
+	t.Run("handles invalid cached type", func(t *testing.T) {
+		provider := &Provider{}
+
+		// Create a test user
+		testUser := &user.User{
+			ID:           999999, // Use a high ID to avoid conflicts
+			AvatarFileID: 0,      // No avatar file ID to avoid actual file operations
+		}
+
+		// Simulate corrupted cached data by storing a string instead of CachedAvatar
+		cacheKey := CacheKeyPrefix + "999999_64"
+		err := keyvalue.Put(cacheKey, "corrupted_string_data")
+		require.NoError(t, err)
+
+		// This should not panic but should handle the type assertion gracefully
+		// and return an error (since there's no actual avatar file)
+		avatar, mimeType, err := provider.GetAvatar(testUser, 64)
+
+		// The function should handle the type assertion failure gracefully
+		// and attempt to regenerate the avatar (which will fail due to no file)
+		require.Error(t, err)
+		assert.Nil(t, avatar)
+		assert.Empty(t, mimeType)
+	})
+
+	t.Run("handles valid cached type", func(t *testing.T) {
+		provider := &Provider{}
+
+		// Create a test user
+		testUser := &user.User{
+			ID:           888888, // Use a different ID to avoid cache conflicts
+			AvatarFileID: 0,
+		}
+
+		// Store a valid cached avatar
+		cacheKey := CacheKeyPrefix + "888888_32"
+		validCachedAvatar := CachedAvatar{
+			Content:  []byte("fake_image_data"),
+			MimeType: "image/png",
+		}
+		err := keyvalue.Put(cacheKey, validCachedAvatar)
+		require.NoError(t, err)
+
+		// This should work correctly with the valid cached data
+		avatar, mimeType, err := provider.GetAvatar(testUser, 32)
+
+		// Should return the cached data successfully
+		require.NoError(t, err)
+		assert.Equal(t, []byte("fake_image_data"), avatar)
+		assert.Equal(t, "image/png", mimeType)
+	})
+}