From fcdcdcf46a2d59ad635c4dda9797889822321c56 Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 17 Jul 2025 15:49:58 +0200 Subject: [PATCH] feat: use keyvalue.Remember where it makes sense --- pkg/models/task_attachment.go | 61 ++++++---------- pkg/modules/auth/openid/providers.go | 15 ++-- pkg/modules/avatar/initials/initials.go | 81 ++++++++------------- pkg/modules/avatar/upload/upload.go | 23 ++---- pkg/modules/background/unsplash/unsplash.go | 20 ++--- pkg/routes/api/v1/task_attachment.go | 15 +--- 6 files changed, 79 insertions(+), 136 deletions(-) diff --git a/pkg/models/task_attachment.go b/pkg/models/task_attachment.go index 200aeaf5d..e4800274f 100644 --- a/pkg/models/task_attachment.go +++ b/pkg/models/task_attachment.go @@ -207,18 +207,36 @@ func cacheKeyForTaskAttachmentPreview(id int64, size PreviewSize) string { return "task_attachment_preview_" + strconv.FormatInt(id, 10) + "_size_" + string(size) } -func (ta *TaskAttachment) GetPreviewFromCache(previewSize PreviewSize) []byte { +func (ta *TaskAttachment) GetPreview(previewSize PreviewSize) []byte { cacheKey := cacheKeyForTaskAttachmentPreview(ta.ID, previewSize) - var cached []byte - exists, err := keyvalue.GetWithValue(cacheKey, &cached) + result, err := keyvalue.Remember(cacheKey, func() (any, error) { + img, _, err := image.Decode(ta.File.File) + if err != nil { + return nil, err + } - // If the preview is not cached, return nil - if err != nil || !exists || cached == nil { + // Scale down the image to a minimum size + resizedImg := resizeImage(img, previewSize.GetSize()) + + // Get the raw bytes of the resized image + buf := &bytes.Buffer{} + if err := png.Encode(buf, resizedImg); err != nil { + return nil, err + } + previewImage, err := io.ReadAll(buf) + if err != nil { + return nil, err + } + + log.Infof("Attachment image preview for task attachment %v of size %v created and cached", ta.ID, previewSize) + return previewImage, nil + }) + if err != nil { return nil } - return cached + return result.([]byte) } type PreviewSize string @@ -276,37 +294,6 @@ func resizeImage(img image.Image, width int) *image.NRGBA { return resizedImg } -func (ta *TaskAttachment) GenerateAndSavePreviewToCache(previewSize PreviewSize) []byte { - img, _, err := image.Decode(ta.File.File) - if err != nil { - return nil - } - - // Scale down the image to a minimum size - resizedImg := resizeImage(img, previewSize.GetSize()) - - // Get the raw bytes of the resized image - buf := &bytes.Buffer{} - if err := png.Encode(buf, resizedImg); err != nil { - return nil - } - previewImage, err := io.ReadAll(buf) - if err != nil { - return nil - } - - // Store the preview image in the cache - cacheKey := cacheKeyForTaskAttachmentPreview(ta.ID, previewSize) - err = keyvalue.Put(cacheKey, previewImage) - if err != nil { - return nil - } - - log.Infof("Attachment image preview for task attachment %v of size %v created and cached", ta.ID, previewSize) - - return previewImage -} - // Delete removes an attachment // @Summary Delete an attachment // @Description Delete an attachment. diff --git a/pkg/modules/auth/openid/providers.go b/pkg/modules/auth/openid/providers.go index d9b3e39e5..441d4662c 100644 --- a/pkg/modules/auth/openid/providers.go +++ b/pkg/modules/auth/openid/providers.go @@ -89,22 +89,23 @@ func GetAllProviders() (providers []*Provider, err error) { // GetProvider retrieves a provider from keyvalue func GetProvider(key string) (provider *Provider, err error) { - provider = &Provider{} - exists, err := keyvalue.GetWithValue("openid_provider_"+key, provider) - if err != nil { - return nil, err - } - if !exists { - _, err = GetAllProviders() // This will put all providers in cache + result, err := keyvalue.Remember("openid_provider_"+key, func() (any, error) { + _, err := GetAllProviders() // This will put all providers in cache if err != nil { return nil, err } + provider := &Provider{} _, err = keyvalue.GetWithValue("openid_provider_"+key, provider) if err != nil { return nil, err } + return provider, nil + }) + if err != nil { + return nil, err } + provider = result.(*Provider) err = provider.setOicdProvider() return diff --git a/pkg/modules/avatar/initials/initials.go b/pkg/modules/avatar/initials/initials.go index 27c22a001..cdf525db6 100644 --- a/pkg/modules/avatar/initials/initials.go +++ b/pkg/modules/avatar/initials/initials.go @@ -135,13 +135,7 @@ func getCacheKey(prefix string, keys ...int64) string { func getAvatarForUser(u *user.User) (fullSizeAvatar *image.RGBA64, err error) { cacheKey := getCacheKey("full", u.ID) - fullSizeAvatar = &image.RGBA64{} - exists, err := keyvalue.GetWithValue(cacheKey, fullSizeAvatar) - if err != nil { - return nil, err - } - - if !exists { + result, err := keyvalue.Remember(cacheKey, func() (any, error) { log.Debugf("Initials avatar for user %d not cached, creating...", u.ID) avatarText := u.Name if avatarText == "" { @@ -150,17 +144,12 @@ func getAvatarForUser(u *user.User) (fullSizeAvatar *image.RGBA64, err error) { firstRune := []rune(strings.ToUpper(avatarText))[0] bg := avatarBgColors[int(u.ID)%len(avatarBgColors)] // Random color based on the user id - fullSizeAvatar, err = drawImage(firstRune, bg) - if err != nil { - return nil, err - } - err = keyvalue.Put(cacheKey, fullSizeAvatar) - if err != nil { - return nil, err - } + return drawImage(firstRune, bg) + }) + if err != nil { + return nil, err } - - return fullSizeAvatar, nil + return result.(*image.RGBA64), nil } // CachedAvatar represents a cached avatar with its content and mime type @@ -173,41 +162,33 @@ type CachedAvatar struct { func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType string, err error) { cacheKey := getCacheKey("resized", u.ID, size) - var cachedAvatar CachedAvatar - exists, err := keyvalue.GetWithValue(cacheKey, &cachedAvatar) + result, err := keyvalue.Remember(cacheKey, func() (any, error) { + log.Debugf("Initials avatar for user %d and size %d not cached, creating...", u.ID, size) + fullAvatar, err := getAvatarForUser(u) + if err != nil { + return nil, err + } + + img := imaging.Resize(fullAvatar, int(size), int(size), imaging.Lanczos) + buf := &bytes.Buffer{} + err = png.Encode(buf, img) + if err != nil { + return nil, err + } + avatar := buf.Bytes() + mimeType := "image/png" + + cachedAvatar := CachedAvatar{ + Content: avatar, + MimeType: mimeType, + } + + return cachedAvatar, nil + }) if err != nil { return nil, "", err } - if exists && len(cachedAvatar.Content) > 0 { - log.Debugf("Serving initials avatar for user %d and size %d from cache", u.ID, size) - return cachedAvatar.Content, cachedAvatar.MimeType, nil - } - - log.Debugf("Initials avatar for user %d and size %d not cached, creating...", u.ID, size) - fullAvatar, err := getAvatarForUser(u) - if err != nil { - return nil, "", err - } - - img := imaging.Resize(fullAvatar, int(size), int(size), imaging.Lanczos) - buf := &bytes.Buffer{} - err = png.Encode(buf, img) - if err != nil { - return nil, "", err - } - avatar = buf.Bytes() - mimeType = "image/png" - - cachedAvatar = CachedAvatar{ - Content: avatar, - MimeType: mimeType, - } - - err = keyvalue.Put(cacheKey, cachedAvatar) - if err != nil { - return nil, "", err - } - - return avatar, mimeType, nil + cachedAvatar := result.(CachedAvatar) + return cachedAvatar.Content, cachedAvatar.MimeType, nil } diff --git a/pkg/modules/avatar/upload/upload.go b/pkg/modules/avatar/upload/upload.go index 113dd92db..30abfc4e8 100644 --- a/pkg/modules/avatar/upload/upload.go +++ b/pkg/modules/avatar/upload/upload.go @@ -51,28 +51,19 @@ type CachedAvatar struct { // GetAvatar returns an uploaded user avatar func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType string, err error) { - cacheKey := CacheKeyPrefix + strconv.Itoa(int(u.ID)) - var cached map[int64]*CachedAvatar - exists, err := keyvalue.GetWithValue(cacheKey, &cached) + result, err := keyvalue.Remember(cacheKey, func() (any, error) { + return make(map[int64]*CachedAvatar), nil + }) if err != nil { return nil, "", err } - if !exists { - // Nothing ever cached for this user so we need to create the size map to avoid panics - cached = make(map[int64]*CachedAvatar) - } else { - a := cached - if a != nil && a[size] != nil { - log.Debugf("Serving uploaded avatar for user %d and size %d from cache.", u.ID, size) - return a[size].Content, a[size].MimeType, nil - } - // This means we have a map for the user, but nothing in it. - if a == nil { - cached = make(map[int64]*CachedAvatar) - } + cached := result.(map[int64]*CachedAvatar) + if cached[size] != nil { + log.Debugf("Serving uploaded avatar for user %d and size %d from cache.", u.ID, size) + return cached[size].Content, cached[size].MimeType, nil } log.Debugf("Uploaded avatar for user %d and size %d not cached, resizing and caching.", u.ID, size) diff --git a/pkg/modules/background/unsplash/unsplash.go b/pkg/modules/background/unsplash/unsplash.go index 24fbd4bc9..e1893a95e 100644 --- a/pkg/modules/background/unsplash/unsplash.go +++ b/pkg/modules/background/unsplash/unsplash.go @@ -122,22 +122,16 @@ func getImageID(fullURL string) string { // Gets an unsplash photo either from cache or directly from the unsplash api func getUnsplashPhotoInfoByID(photoID string) (photo *Photo, err error) { - - photo = &Photo{} - exists, err := keyvalue.GetWithValue(cachePrefix+photoID, photo) + result, err := keyvalue.Remember(cachePrefix+photoID, func() (any, error) { + log.Debugf("Image information for unsplash photo %s not cached, requesting from unsplash...", photoID) + photo := &Photo{} + err := doGet("photos/"+photoID, photo) + return photo, err + }) if err != nil { return nil, err } - - if !exists { - log.Debugf("Image information for unsplash photo %s not cached, requesting from unsplash...", photoID) - photo = &Photo{} - err = doGet("photos/"+photoID, photo) - if err != nil { - return - } - } - return + return result.(*Photo), nil } // Search is the implementation to search on unsplash diff --git a/pkg/routes/api/v1/task_attachment.go b/pkg/routes/api/v1/task_attachment.go index 5b86aa369..01db5dbb8 100644 --- a/pkg/routes/api/v1/task_attachment.go +++ b/pkg/routes/api/v1/task_attachment.go @@ -22,7 +22,6 @@ import ( "strings" "code.vikunja.io/api/pkg/db" - "code.vikunja.io/api/pkg/log" "code.vikunja.io/api/pkg/models" auth2 "code.vikunja.io/api/pkg/modules/auth" "code.vikunja.io/api/pkg/web/handler" @@ -160,13 +159,11 @@ func GetTaskAttachment(c echo.Context) error { return handler.HandleHTTPError(err) } - // If the preview query parameter is set and the preview was already generated and cached, return the cached preview image + // If the preview query parameter is set, get the preview (cached or generate) previewSize := models.GetPreviewSizeFromString(c.QueryParam("preview_size")) if previewSize != models.PreviewSizeUnknown && strings.HasPrefix(taskAttachment.File.Mime, "image") { - previewFileBytes := taskAttachment.GetPreviewFromCache(previewSize) + previewFileBytes := taskAttachment.GetPreview(previewSize) if previewFileBytes != nil { - log.Debugf("Cached attachment image preview found for task attachment %v", taskAttachment.ID) - return c.Blob(http.StatusOK, "image/png", previewFileBytes) } } @@ -183,14 +180,6 @@ func GetTaskAttachment(c echo.Context) error { return handler.HandleHTTPError(err) } - // If a preview is requested and the preview was not cached, we create the preview and cache it - if previewSize != models.PreviewSizeUnknown { - previewFileBytes := taskAttachment.GenerateAndSavePreviewToCache(previewSize) - if previewFileBytes != nil { - return c.Blob(http.StatusOK, "image/png", previewFileBytes) - } - } - http.ServeContent(c.Response(), c.Request(), taskAttachment.File.Name, taskAttachment.File.Created, taskAttachment.File.File) return nil }