[feature] do not uncache status / emoji media if attached status is bookmarked (#2956)

* do not uncache status / emoji media if attached status is bookmarked

* add status bookmark and bookmark IDs caches

* update status bookmark tests

* move IsStatusBookmarkedBy() to StatusBookmark{} interface, rely on cache

* fix envparsing.sh test

internal/cache/cache.go

@@ -85,6 +85,8 @@ func (c *Caches) Init() {
 	c.initPollVoteIDs()
 	c.initReport()
 	c.initStatus()
+	c.initStatusBookmark()
+	c.initStatusBookmarkIDs()
 	c.initStatusFave()
 	c.initStatusFaveIDs()
 	c.initTag()
@@ -101,7 +103,7 @@ func (c *Caches) Init() {
 func (c *Caches) Start() {
 	log.Infof(nil, "start: %p", c)
 
-	tryUntil("starting *gtsmodel.Webfinger cache", 5, func() bool {
+	tryUntil("starting webfinger cache", 5, func() bool {
 		return c.GTS.Webfinger.Start(5 * time.Minute)
 	})
 }
@@ -111,7 +113,7 @@ func (c *Caches) Start() {
 func (c *Caches) Stop() {
 	log.Infof(nil, "stop: %p", c)
 
-	tryUntil("stopping *gtsmodel.Webfinger cache", 5, c.GTS.Webfinger.Stop)
+	tryUntil("stopping webfinger cache", 5, c.GTS.Webfinger.Stop)
 }
 
 // Sweep will sweep all the available caches to ensure none
@@ -153,6 +155,8 @@ func (c *Caches) Sweep(threshold float64) {
 	c.GTS.PollVoteIDs.Trim(threshold)
 	c.GTS.Report.Trim(threshold)
 	c.GTS.Status.Trim(threshold)
+	c.GTS.StatusBookmark.Trim(threshold)
+	c.GTS.StatusBookmarkIDs.Trim(threshold)
 	c.GTS.StatusFave.Trim(threshold)
 	c.GTS.StatusFaveIDs.Trim(threshold)
 	c.GTS.Tag.Trim(threshold)

internal/cache/db.go

@@ -139,6 +139,12 @@ type GTSCaches struct {
 	// Status provides access to the gtsmodel Status database cache.
 	Status StructCache[*gtsmodel.Status]
 
+	// StatusBookmark ...
+	StatusBookmark StructCache[*gtsmodel.StatusBookmark]
+
+	// StatusBookmarkIDs ...
+	StatusBookmarkIDs SliceCache[string]
+
 	// StatusFave provides access to the gtsmodel StatusFave database cache.
 	StatusFave StructCache[*gtsmodel.StatusFave]
 
@@ -1102,6 +1108,54 @@ func (c *Caches) initStatus() {
 	})
 }
 
+func (c *Caches) initStatusBookmark() {
+	// Calculate maximum cache size.
+	cap := calculateResultCacheMax(
+		sizeofStatusBookmark(), // model in-mem size.
+		config.GetCacheStatusBookmarkMemRatio(),
+	)
+
+	log.Infof(nil, "cache size = %d", cap)
+
+	copyF := func(s1 *gtsmodel.StatusBookmark) *gtsmodel.StatusBookmark {
+		s2 := new(gtsmodel.StatusBookmark)
+		*s2 = *s1
+
+		// Don't include ptr fields that
+		// will be populated separately.
+		s2.Account = nil
+		s2.TargetAccount = nil
+		s2.Status = nil
+
+		return s2
+	}
+
+	c.GTS.StatusBookmark.Init(structr.CacheConfig[*gtsmodel.StatusBookmark]{
+		Indices: []structr.IndexConfig{
+			{Fields: "ID"},
+			{Fields: "AccountID,StatusID"},
+			{Fields: "AccountID", Multiple: true},
+			{Fields: "TargetAccountID", Multiple: true},
+			{Fields: "StatusID", Multiple: true},
+		},
+		MaxSize:    cap,
+		IgnoreErr:  ignoreErrors,
+		Copy:       copyF,
+		Invalidate: c.OnInvalidateStatusBookmark,
+	})
+}
+
+func (c *Caches) initStatusBookmarkIDs() {
+	// Calculate maximum cache size.
+	cap := calculateSliceCacheMax(
+		config.GetCacheStatusBookmarkIDsMemRatio(),
+	)
+
+	log.Infof(nil, "cache size = %d", cap)
+
+	c.GTS.StatusBookmarkIDs.Init(0, cap)
+}
+
 func (c *Caches) initStatusFave() {
 	// Calculate maximum cache size.
 	cap := calculateResultCacheMax(

internal/cache/invalidate.go

@@ -198,6 +198,11 @@ func (c *Caches) OnInvalidateStatus(status *gtsmodel.Status) {
 	}
 }
 
+func (c *Caches) OnInvalidateStatusBookmark(bookmark *gtsmodel.StatusBookmark) {
+	// Invalidate status bookmark ID list for this status.
+	c.GTS.StatusBookmarkIDs.Invalidate(bookmark.StatusID)
+}
+
 func (c *Caches) OnInvalidateStatusFave(fave *gtsmodel.StatusFave) {
 	// Invalidate status fave ID list for this status.
 	c.GTS.StatusFaveIDs.Invalidate(fave.StatusID)

internal/cache/size.go

@@ -201,6 +201,8 @@ func totalOfRatios() float64 {
 		config.GetCachePollVoteMemRatio() +
 		config.GetCacheReportMemRatio() +
 		config.GetCacheStatusMemRatio() +
+		config.GetCacheStatusBookmarkMemRatio() +
+		config.GetCacheStatusBookmarkIDsMemRatio() +
 		config.GetCacheStatusFaveMemRatio() +
 		config.GetCacheStatusFaveIDsMemRatio() +
 		config.GetCacheTagMemRatio() +
@@ -566,7 +568,7 @@ func sizeofReport() uintptr {
 
 func sizeofStatus() uintptr {
 	return uintptr(size.Of(&gtsmodel.Status{
-		ID:                       exampleURI,
+		ID:                       exampleID,
 		URI:                      exampleURI,
 		URL:                      exampleURI,
 		Content:                  exampleText,
@@ -599,6 +601,20 @@ func sizeofStatus() uintptr {
 	}))
 }
 
+func sizeofStatusBookmark() uintptr {
+	return uintptr(size.Of(&gtsmodel.StatusBookmark{
+		ID:              exampleID,
+		AccountID:       exampleID,
+		Account:         nil,
+		TargetAccountID: exampleID,
+		TargetAccount:   nil,
+		StatusID:        exampleID,
+		Status:          nil,
+		CreatedAt:       exampleTime,
+		UpdatedAt:       exampleTime,
+	}))
+}
+
 func sizeofStatusFave() uintptr {
 	return uintptr(size.Of(&gtsmodel.StatusFave{
 		ID:              exampleID,

internal/cleaner/emoji.go

@@ -32,9 +32,7 @@ import (
 
 // Emoji encompasses a set of
 // emoji cleanup / admin utils.
-type Emoji struct {
+type Emoji struct{ *Cleaner }
-	*Cleaner
-}
 
 // All will execute all cleaner.Emoji utilities synchronously, including output logging.
 // Context will be checked for `gtscontext.DryRun()` in order to actually perform the action.
@@ -381,10 +379,20 @@ func (e *Emoji) uncacheRemote(ctx context.Context, after time.Time, emoji *gtsmo
 	}
 
 	for _, status := range statuses {
+		// Check if recently used status.
 		if status.FetchedAt.After(after) {
 			l.Debug("skipping due to recently fetched status")
 			return false, nil
 		}
+
+		// Check whether status is bookmarked by active accounts.
+		bookmarked, err := e.state.DB.IsStatusBookmarked(ctx, status.ID)
+		if err != nil {
+			return false, err
+		} else if bookmarked {
+			l.Debug("skipping due to bookmarked status")
+			return false, nil
+		}
 	}
 
 	// This emoji is too old, uncache it.

internal/cleaner/media.go

@@ -35,9 +35,7 @@ import (
 
 // Media encompasses a set of
 // media cleanup / admin utils.
-type Media struct {
+type Media struct{ *Cleaner }
-	*Cleaner
-}
 
 // All will execute all cleaner.Media utilities synchronously, including output logging.
 // Context will be checked for `gtscontext.DryRun()` in order to actually perform the action.
@@ -475,10 +473,22 @@ func (m *Media) uncacheRemote(ctx context.Context, after time.Time, media *gtsmo
 			return false, nil
 		}
 
-		if status != nil && status.FetchedAt.After(after) {
+		if status != nil {
+			// Check if recently used status.
+			if status.FetchedAt.After(after) {
 				l.Debug("skipping due to recently fetched status")
 				return false, nil
 			}
+
+			// Check whether status is bookmarked by active accounts.
+			bookmarked, err := m.state.DB.IsStatusBookmarked(ctx, status.ID)
+			if err != nil {
+				return false, err
+			} else if bookmarked {
+				l.Debug("skipping due to bookmarked status")
+				return false, nil
+			}
+		}
 	}
 
 	// This media is too old, uncache it.

internal/config/config.go

@@ -224,6 +224,8 @@ type CacheConfiguration struct {
 	PollVoteIDsMemRatio       float64       `name:"poll-vote-ids-mem-ratio"`
 	ReportMemRatio            float64       `name:"report-mem-ratio"`
 	StatusMemRatio            float64       `name:"status-mem-ratio"`
+	StatusBookmarkMemRatio    float64       `name:"status-bookmark-mem-ratio"`
+	StatusBookmarkIDsMemRatio float64       `name:"status-bookmark-ids-mem-ratio"`
 	StatusFaveMemRatio        float64       `name:"status-fave-mem-ratio"`
 	StatusFaveIDsMemRatio     float64       `name:"status-fave-ids-mem-ratio"`
 	TagMemRatio               float64       `name:"tag-mem-ratio"`

internal/config/defaults.go

@@ -188,6 +188,8 @@ var Defaults = Configuration{
 		PollVoteIDsMemRatio:       2,
 		ReportMemRatio:            1,
 		StatusMemRatio:            5,
+		StatusBookmarkMemRatio:    0.5,
+		StatusBookmarkIDsMemRatio: 2,
 		StatusFaveMemRatio:        2,
 		StatusFaveIDsMemRatio:     3,
 		TagMemRatio:               2,

internal/config/helpers.gen.go

@@ -3550,6 +3550,56 @@ func GetCacheStatusMemRatio() float64 { return global.GetCacheStatusMemRatio() }
 // SetCacheStatusMemRatio safely sets the value for global configuration 'Cache.StatusMemRatio' field
 func SetCacheStatusMemRatio(v float64) { global.SetCacheStatusMemRatio(v) }
 
+// GetCacheStatusBookmarkMemRatio safely fetches the Configuration value for state's 'Cache.StatusBookmarkMemRatio' field
+func (st *ConfigState) GetCacheStatusBookmarkMemRatio() (v float64) {
+	st.mutex.RLock()
+	v = st.config.Cache.StatusBookmarkMemRatio
+	st.mutex.RUnlock()
+	return
+}
+
+// SetCacheStatusBookmarkMemRatio safely sets the Configuration value for state's 'Cache.StatusBookmarkMemRatio' field
+func (st *ConfigState) SetCacheStatusBookmarkMemRatio(v float64) {
+	st.mutex.Lock()
+	defer st.mutex.Unlock()
+	st.config.Cache.StatusBookmarkMemRatio = v
+	st.reloadToViper()
+}
+
+// CacheStatusBookmarkMemRatioFlag returns the flag name for the 'Cache.StatusBookmarkMemRatio' field
+func CacheStatusBookmarkMemRatioFlag() string { return "cache-status-bookmark-mem-ratio" }
+
+// GetCacheStatusBookmarkMemRatio safely fetches the value for global configuration 'Cache.StatusBookmarkMemRatio' field
+func GetCacheStatusBookmarkMemRatio() float64 { return global.GetCacheStatusBookmarkMemRatio() }
+
+// SetCacheStatusBookmarkMemRatio safely sets the value for global configuration 'Cache.StatusBookmarkMemRatio' field
+func SetCacheStatusBookmarkMemRatio(v float64) { global.SetCacheStatusBookmarkMemRatio(v) }
+
+// GetCacheStatusBookmarkIDsMemRatio safely fetches the Configuration value for state's 'Cache.StatusBookmarkIDsMemRatio' field
+func (st *ConfigState) GetCacheStatusBookmarkIDsMemRatio() (v float64) {
+	st.mutex.RLock()
+	v = st.config.Cache.StatusBookmarkIDsMemRatio
+	st.mutex.RUnlock()
+	return
+}
+
+// SetCacheStatusBookmarkIDsMemRatio safely sets the Configuration value for state's 'Cache.StatusBookmarkIDsMemRatio' field
+func (st *ConfigState) SetCacheStatusBookmarkIDsMemRatio(v float64) {
+	st.mutex.Lock()
+	defer st.mutex.Unlock()
+	st.config.Cache.StatusBookmarkIDsMemRatio = v
+	st.reloadToViper()
+}
+
+// CacheStatusBookmarkIDsMemRatioFlag returns the flag name for the 'Cache.StatusBookmarkIDsMemRatio' field
+func CacheStatusBookmarkIDsMemRatioFlag() string { return "cache-status-bookmark-ids-mem-ratio" }
+
+// GetCacheStatusBookmarkIDsMemRatio safely fetches the value for global configuration 'Cache.StatusBookmarkIDsMemRatio' field
+func GetCacheStatusBookmarkIDsMemRatio() float64 { return global.GetCacheStatusBookmarkIDsMemRatio() }
+
+// SetCacheStatusBookmarkIDsMemRatio safely sets the value for global configuration 'Cache.StatusBookmarkIDsMemRatio' field
+func SetCacheStatusBookmarkIDsMemRatio(v float64) { global.SetCacheStatusBookmarkIDsMemRatio(v) }
+
 // GetCacheStatusFaveMemRatio safely fetches the Configuration value for state's 'Cache.StatusFaveMemRatio' field
 func (st *ConfigState) GetCacheStatusFaveMemRatio() (v float64) {
 	st.mutex.RLock()

internal/db/bundb/status.go

@@ -677,12 +677,3 @@ func (s *statusDB) getStatusBoostIDs(ctx context.Context, statusID string) ([]st
 		return statusIDs, nil
 	})
 }
-
-func (s *statusDB) IsStatusBookmarkedBy(ctx context.Context, status *gtsmodel.Status, accountID string) (bool, error) {
-	q := s.db.
-		NewSelect().
-		TableExpr("? AS ?", bun.Ident("status_bookmarks"), bun.Ident("status_bookmark")).
-		Where("? = ?", bun.Ident("status_bookmark.status_id"), status.ID).
-		Where("? = ?", bun.Ident("status_bookmark.account_id"), accountID)
-	return exists(ctx, q)
-}

internal/db/bundb/statusbookmark.go

@@ -20,11 +20,15 @@ package bundb
 import (
 	"context"
 	"errors"
-	"fmt"
+	"slices"
 
+	"github.com/superseriousbusiness/gotosocial/internal/db"
+	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
+	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/log"
 	"github.com/superseriousbusiness/gotosocial/internal/state"
+	"github.com/superseriousbusiness/gotosocial/internal/util"
 	"github.com/uptrace/bun"
 )
 
@@ -33,52 +37,158 @@ type statusBookmarkDB struct {
 	state *state.State
 }
 
-func (s *statusBookmarkDB) GetStatusBookmark(ctx context.Context, id string) (*gtsmodel.StatusBookmark, error) {
+func (s *statusBookmarkDB) GetStatusBookmarkByID(ctx context.Context, id string) (*gtsmodel.StatusBookmark, error) {
-	bookmark := new(gtsmodel.StatusBookmark)
+	return s.getStatusBookmark(
-
+		ctx,
-	err := s.db.
+		"ID",
+		func(bookmark *gtsmodel.StatusBookmark) error {
+			return s.db.
 				NewSelect().
 				Model(bookmark).
-		Where("? = ?", bun.Ident("status_bookmark.id"), id).
+				Where("? = ?", bun.Ident("id"), id).
 				Scan(ctx)
+		},
+		id,
+	)
+}
+
+func (s *statusBookmarkDB) GetStatusBookmark(ctx context.Context, accountID string, statusID string) (*gtsmodel.StatusBookmark, error) {
+	return s.getStatusBookmark(
+		ctx,
+		"AccountID,StatusID",
+		func(bookmark *gtsmodel.StatusBookmark) error {
+			return s.db.
+				NewSelect().
+				Model(bookmark).
+				Where("? = ?", bun.Ident("account_id"), accountID).
+				Where("? = ?", bun.Ident("status_id"), statusID).
+				Scan(ctx)
+		},
+		accountID, statusID,
+	)
+}
+
+func (s *statusBookmarkDB) GetStatusBookmarksByIDs(ctx context.Context, ids []string) ([]*gtsmodel.StatusBookmark, error) {
+	// Load all input bookmark IDs via cache loader callback.
+	bookmarks, err := s.state.Caches.GTS.StatusBookmark.LoadIDs("ID",
+		ids,
+		func(uncached []string) ([]*gtsmodel.StatusBookmark, error) {
+			// Preallocate expected length of uncached bookmarks.
+			bookmarks := make([]*gtsmodel.StatusBookmark, 0, len(uncached))
+
+			// Perform database query scanning
+			// the remaining (uncached) bookmarks.
+			if err := s.db.NewSelect().
+				Model(&bookmarks).
+				Where("? IN (?)", bun.Ident("id"), bun.In(uncached)).
+				Scan(ctx); err != nil {
+				return nil, err
+			}
+
+			return bookmarks, nil
+		},
+	)
 	if err != nil {
 		return nil, err
 	}
 
-	bookmark.Account, err = s.state.DB.GetAccountByID(ctx, bookmark.AccountID)
+	// Reorder the bookmarks by their
-	if err != nil {
+	// IDs to ensure in correct order.
-		return nil, fmt.Errorf("error getting status bookmark account %q: %w", bookmark.AccountID, err)
+	getID := func(b *gtsmodel.StatusBookmark) string { return b.ID }
+	util.OrderBy(bookmarks, ids, getID)
+
+	// Populate all loaded bookmarks, removing those we fail
+	// to populate (removes needing so many later nil checks).
+	bookmarks = slices.DeleteFunc(bookmarks, func(bookmark *gtsmodel.StatusBookmark) bool {
+		if err := s.PopulateStatusBookmark(ctx, bookmark); err != nil {
+			log.Errorf(ctx, "error populating bookmark %s: %v", bookmark.ID, err)
+			return true
+		}
+		return false
+	})
+
+	return bookmarks, nil
 }
 
-	bookmark.TargetAccount, err = s.state.DB.GetAccountByID(ctx, bookmark.TargetAccountID)
+func (s *statusBookmarkDB) IsStatusBookmarked(ctx context.Context, statusID string) (bool, error) {
-	if err != nil {
+	bookmarkIDs, err := s.getStatusBookmarkIDs(ctx, statusID)
-		return nil, fmt.Errorf("error getting status bookmark target account %q: %w", bookmark.TargetAccountID, err)
+	return (len(bookmarkIDs) > 0), err
 }
 
-	bookmark.Status, err = s.state.DB.GetStatusByID(ctx, bookmark.StatusID)
+func (s *statusBookmarkDB) IsStatusBookmarkedBy(ctx context.Context, accountID string, statusID string) (bool, error) {
+	bookmark, err := s.GetStatusBookmark(ctx, accountID, statusID)
+	if err != nil && !errors.Is(err, db.ErrNoEntries) {
+		return false, err
+	}
+	return (bookmark != nil), nil
+}
+
+func (s *statusBookmarkDB) getStatusBookmark(ctx context.Context, lookup string, dbQuery func(*gtsmodel.StatusBookmark) error, keyParts ...any) (*gtsmodel.StatusBookmark, error) {
+	// Fetch bookmark from database cache with loader callback.
+	bookmark, err := s.state.Caches.GTS.StatusBookmark.LoadOne(lookup, func() (*gtsmodel.StatusBookmark, error) {
+		var bookmark gtsmodel.StatusBookmark
+
+		// Not cached! Perform database query.
+		if err := dbQuery(&bookmark); err != nil {
+			return nil, err
+		}
+
+		return &bookmark, nil
+	}, keyParts...)
 	if err != nil {
-		return nil, fmt.Errorf("error getting status bookmark status %q: %w", bookmark.StatusID, err)
+		return nil, err
+	}
+
+	if gtscontext.Barebones(ctx) {
+		// no need to fully populate.
+		return bookmark, nil
+	}
+
+	// Further populate the bookmark fields where applicable.
+	if err := s.PopulateStatusBookmark(ctx, bookmark); err != nil {
+		return nil, err
 	}
 
 	return bookmark, nil
 }
 
-func (s *statusBookmarkDB) GetStatusBookmarkID(ctx context.Context, accountID string, statusID string) (string, error) {
+func (s *statusBookmarkDB) PopulateStatusBookmark(ctx context.Context, bookmark *gtsmodel.StatusBookmark) (err error) {
-	var id string
+	var errs gtserror.MultiError
 
-	q := s.db.
+	if bookmark.Account == nil {
-		NewSelect().
+		// Bookmark author is not set, fetch from database.
-		TableExpr("? AS ?", bun.Ident("status_bookmarks"), bun.Ident("status_bookmark")).
+		bookmark.Account, err = s.state.DB.GetAccountByID(
-		Column("status_bookmark.id").
+			gtscontext.SetBarebones(ctx),
-		Where("? = ?", bun.Ident("status_bookmark.account_id"), accountID).
+			bookmark.AccountID,
-		Where("? = ?", bun.Ident("status_bookmark.status_id"), statusID).
+		)
-		Limit(1)
+		if err != nil {
-
+			errs.Appendf("error getting bookmark account %s: %w", bookmark.AccountID, err)
-	if err := q.Scan(ctx, &id); err != nil {
+		}
-		return "", err
 	}
 
-	return id, nil
+	if bookmark.TargetAccount == nil {
+		// Bookmark target account is not set, fetch from database.
+		bookmark.TargetAccount, err = s.state.DB.GetAccountByID(
+			gtscontext.SetBarebones(ctx),
+			bookmark.TargetAccountID,
+		)
+		if err != nil {
+			errs.Appendf("error getting bookmark target account %s: %w", bookmark.TargetAccountID, err)
+		}
+	}
+
+	if bookmark.Status == nil {
+		// Bookmarked status not set, fetch from database.
+		bookmark.Status, err = s.state.DB.GetStatusByID(
+			gtscontext.SetBarebones(ctx),
+			bookmark.StatusID,
+		)
+		if err != nil {
+			errs.Appendf("error getting bookmark status %s: %w", bookmark.StatusID, err)
+		}
+	}
+
+	return errs.Combine()
 }
 
 func (s *statusBookmarkDB) GetStatusBookmarks(ctx context.Context, accountID string, limit int, maxID string, minID string) ([]*gtsmodel.StatusBookmark, error) {
@@ -117,81 +227,90 @@ func (s *statusBookmarkDB) GetStatusBookmarks(ctx context.Context, accountID str
 		return nil, err
 	}
 
-	bookmarks := make([]*gtsmodel.StatusBookmark, 0, len(ids))
+	return s.GetStatusBookmarksByIDs(ctx, ids)
-
-	for _, id := range ids {
-		bookmark, err := s.GetStatusBookmark(ctx, id)
-		if err != nil {
-			log.Errorf(ctx, "error getting bookmark %q: %v", id, err)
-			continue
 }
 
-		bookmarks = append(bookmarks, bookmark)
+func (s *statusBookmarkDB) getStatusBookmarkIDs(ctx context.Context, statusID string) ([]string, error) {
+	return s.state.Caches.GTS.StatusBookmarkIDs.Load(statusID, func() ([]string, error) {
+		var bookmarkIDs []string
+
+		// Bookmark IDs not cached,
+		// perform database query.
+		if err := s.db.
+			NewSelect().
+			Table("status_bookmarks").
+			Column("id").Where("? = ?", bun.Ident("status_id"), statusID).
+			Order("id DESC").
+			Scan(ctx, &bookmarkIDs); err != nil {
+			return nil, err
 		}
 
-	return bookmarks, nil
+		return bookmarkIDs, nil
+	})
 }
 
-func (s *statusBookmarkDB) PutStatusBookmark(ctx context.Context, statusBookmark *gtsmodel.StatusBookmark) error {
+func (s *statusBookmarkDB) PutStatusBookmark(ctx context.Context, bookmark *gtsmodel.StatusBookmark) error {
-	_, err := s.db.
+	return s.state.Caches.GTS.StatusBookmark.Store(bookmark, func() error {
-		NewInsert().
+		_, err := s.db.NewInsert().Model(bookmark).Exec(ctx)
-		Model(statusBookmark).
-		Exec(ctx)
-
 		return err
+	})
 }
 
-func (s *statusBookmarkDB) DeleteStatusBookmark(ctx context.Context, id string) error {
+func (s *statusBookmarkDB) DeleteStatusBookmarkByID(ctx context.Context, id string) error {
 	_, err := s.db.
 		NewDelete().
-		TableExpr("? AS ?", bun.Ident("status_bookmarks"), bun.Ident("status_bookmark")).
+		Table("status_bookmarks").
-		Where("? = ?", bun.Ident("status_bookmark.id"), id).
+		Where("? = ?", bun.Ident("id"), id).
 		Exec(ctx)
-
+	if err != nil {
 		return err
 	}
+	s.state.Caches.GTS.StatusBookmark.Invalidate("ID", id)
+	return nil
+}
 
 func (s *statusBookmarkDB) DeleteStatusBookmarks(ctx context.Context, targetAccountID string, originAccountID string) error {
 	if targetAccountID == "" && originAccountID == "" {
 		return errors.New("DeleteBookmarks: one of targetAccountID or originAccountID must be set")
 	}
 
-	// TODO: Capture bookmark IDs in a RETURNING
-	// statement (when bookmarks have a cache),
-	// + use the IDs to invalidate cache entries.
-
 	q := s.db.
 		NewDelete().
 		TableExpr("? AS ?", bun.Ident("status_bookmarks"), bun.Ident("status_bookmark"))
 
 	if targetAccountID != "" {
 		q = q.Where("? = ?", bun.Ident("status_bookmark.target_account_id"), targetAccountID)
+		defer s.state.Caches.GTS.StatusBookmark.Invalidate("TargetAccountID", targetAccountID)
 	}
 
 	if originAccountID != "" {
 		q = q.Where("? = ?", bun.Ident("status_bookmark.account_id"), originAccountID)
+		defer s.state.Caches.GTS.StatusBookmark.Invalidate("AccountID", originAccountID)
 	}
 
 	if _, err := q.Exec(ctx); err != nil {
 		return err
 	}
 
+	if targetAccountID != "" {
+		s.state.Caches.GTS.StatusBookmark.Invalidate("TargetAccountID", targetAccountID)
+	}
+
+	if originAccountID != "" {
+		s.state.Caches.GTS.StatusBookmark.Invalidate("AccountID", originAccountID)
+	}
+
 	return nil
 }
 
 func (s *statusBookmarkDB) DeleteStatusBookmarksForStatus(ctx context.Context, statusID string) error {
-	// TODO: Capture bookmark IDs in a RETURNING
-	// statement (when bookmarks have a cache),
-	// + use the IDs to invalidate cache entries.
-
 	q := s.db.
 		NewDelete().
 		TableExpr("? AS ?", bun.Ident("status_bookmarks"), bun.Ident("status_bookmark")).
 		Where("? = ?", bun.Ident("status_bookmark.status_id"), statusID)
-
 	if _, err := q.Exec(ctx); err != nil {
 		return err
 	}
-
+	s.state.Caches.GTS.StatusBookmark.Invalidate("StatusID", statusID)
 	return nil
 }

internal/db/bundb/statusbookmark_test.go

@@ -31,23 +31,32 @@ type StatusBookmarkTestSuite struct {
 	BunDBStandardTestSuite
 }
 
-func (suite *StatusBookmarkTestSuite) TestGetStatusBookmarkIDOK() {
+func (suite *StatusBookmarkTestSuite) TestGetStatusBookmarkOK() {
 	testBookmark := suite.testBookmarks["local_account_1_admin_account_status_1"]
-
+	bookmark, err := suite.db.GetStatusBookmark(context.Background(), testBookmark.AccountID, testBookmark.StatusID)
-	id, err := suite.db.GetStatusBookmarkID(context.Background(), testBookmark.AccountID, testBookmark.StatusID)
+	suite.NoError(err)
-	if err != nil {
+	suite.Equal(testBookmark.ID, bookmark.ID)
-		suite.FailNow(err.Error())
+	suite.Equal(testBookmark.AccountID, bookmark.AccountID)
+	suite.Equal(testBookmark.StatusID, bookmark.StatusID)
 }
 
-	suite.Equal(testBookmark.ID, id)
+func (suite *StatusBookmarkTestSuite) TestGetStatusBookmarkNonexisting() {
-}
+	bookmark, err := suite.db.GetStatusBookmark(context.Background(), "01GVAVGD06YJ2FSB5GJSMF8M2K", "01GVAVGKGR1MK9ZN7JCJFYSFZV")
-
+	suite.Nil(bookmark)
-func (suite *StatusBookmarkTestSuite) TestGetStatusBookmarkIDNonexisting() {
-	id, err := suite.db.GetStatusBookmarkID(context.Background(), "01GVAVGD06YJ2FSB5GJSMF8M2K", "01GVAVGKGR1MK9ZN7JCJFYSFZV")
-	suite.Empty(id)
 	suite.ErrorIs(err, db.ErrNoEntries)
 }
 
+func (suite *StatusBookmarkTestSuite) IsStatusBookmarked() {
+	for _, bookmark := range suite.testBookmarks {
+		ok, err := suite.db.IsStatusBookmarked(
+			context.Background(),
+			bookmark.StatusID,
+		)
+		suite.NoError(err)
+		suite.True(ok)
+	}
+}
+
 func (suite *StatusBookmarkTestSuite) TestDeleteStatusBookmarksOriginatingFromAccount() {
 	testAccount := suite.testAccounts["local_account_1"]
 
@@ -105,21 +114,21 @@ func (suite *StatusBookmarkTestSuite) TestDeleteStatusBookmarksTargetingStatus()
 	}
 }
 
-func (suite *StatusBookmarkTestSuite) TestDeleteStatusBookmark() {
+func (suite *StatusBookmarkTestSuite) TestDeleteStatusBookmarkByID() {
 	testBookmark := suite.testBookmarks["local_account_1_admin_account_status_1"]
 	ctx := context.Background()
 
-	if err := suite.db.DeleteStatusBookmark(ctx, testBookmark.ID); err != nil {
+	if err := suite.db.DeleteStatusBookmarkByID(ctx, testBookmark.ID); err != nil {
 		suite.FailNow(err.Error())
 	}
 
-	bookmark, err := suite.db.GetStatusBookmark(ctx, testBookmark.ID)
+	bookmark, err := suite.db.GetStatusBookmarkByID(ctx, testBookmark.ID)
 	suite.ErrorIs(err, db.ErrNoEntries)
 	suite.Nil(bookmark)
 }
 
-func (suite *StatusBookmarkTestSuite) TestDeleteStatusBookmarkNonExisting() {
+func (suite *StatusBookmarkTestSuite) TestDeleteStatusBookmarkByIDNonExisting() {
-	err := suite.db.DeleteStatusBookmark(context.Background(), "01GVAV715K6Y2SG9ZKS9ZA8G7G")
+	err := suite.db.DeleteStatusBookmarkByID(context.Background(), "01GVAV715K6Y2SG9ZKS9ZA8G7G")
 	suite.NoError(err)
 }
 

internal/db/status.go

@@ -78,7 +78,4 @@ type Status interface {
 
 	// GetStatusChildren gets the child statuses of a given status.
 	GetStatusChildren(ctx context.Context, statusID string) ([]*gtsmodel.Status, error)
-
-	// IsStatusBookmarkedBy checks if a given status has been bookmarked by a given account ID
-	IsStatusBookmarkedBy(ctx context.Context, status *gtsmodel.Status, accountID string) (bool, error)
 }

internal/db/statusbookmark.go

@@ -25,11 +25,16 @@ import (
 
 type StatusBookmark interface {
 	// GetStatusBookmark gets one status bookmark with the given ID.
-	GetStatusBookmark(ctx context.Context, id string) (*gtsmodel.StatusBookmark, error)
+	GetStatusBookmarkByID(ctx context.Context, id string) (*gtsmodel.StatusBookmark, error)
 
-	// GetStatusBookmarkID is a shortcut function for returning just the database ID
+	// GetStatusBookmark fetches a status bookmark by the given account ID on the given status ID, if it exists.
-	// of a status bookmark created by the given accountID, targeting the given statusID.
+	GetStatusBookmark(ctx context.Context, accountID string, statusID string) (*gtsmodel.StatusBookmark, error)
-	GetStatusBookmarkID(ctx context.Context, accountID string, statusID string) (string, error)
+
+	// IsStatusBookmarked returns whether status has been bookmarked by any account.
+	IsStatusBookmarked(ctx context.Context, statusID string) (bool, error)
+
+	// IsStatusBookmarkedBy returns whether status ID is bookmarked by the given account ID.
+	IsStatusBookmarkedBy(ctx context.Context, accountID string, statusID string) (bool, error)
 
 	// GetStatusBookmarks retrieves status bookmarks created by the given accountID,
 	// and using the provided parameters. If limit is < 0 then no limit will be set.
@@ -42,7 +47,7 @@ type StatusBookmark interface {
 	PutStatusBookmark(ctx context.Context, statusBookmark *gtsmodel.StatusBookmark) error
 
 	// DeleteStatusBookmark deletes one status bookmark with the given ID.
-	DeleteStatusBookmark(ctx context.Context, id string) error
+	DeleteStatusBookmarkByID(ctx context.Context, id string) error
 
 	// DeleteStatusBookmarks mass deletes status bookmarks targeting targetAccountID
 	// and/or originating from originAccountID and/or bookmarking statusID.

internal/processing/status/bookmark.go

@@ -20,42 +20,23 @@ package status
 import (
 	"context"
 	"errors"
-	"fmt"
 
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
 	"github.com/superseriousbusiness/gotosocial/internal/db"
+	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/id"
 )
 
-func (p *Processor) getBookmarkableStatus(ctx context.Context, requestingAccount *gtsmodel.Account, targetStatusID string) (*gtsmodel.Status, string, gtserror.WithCode) {
-	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
-		requestingAccount,
-		targetStatusID,
-		nil, // default freshness
-	)
-	if errWithCode != nil {
-		return nil, "", errWithCode
-	}
-
-	bookmarkID, err := p.state.DB.GetStatusBookmarkID(ctx, requestingAccount.ID, targetStatus.ID)
-	if err != nil && !errors.Is(err, db.ErrNoEntries) {
-		err = fmt.Errorf("getBookmarkTarget: error checking existing bookmark: %w", err)
-		return nil, "", gtserror.NewErrorInternalError(err)
-	}
-
-	return targetStatus, bookmarkID, nil
-}
-
 // BookmarkCreate adds a bookmark for the requestingAccount, targeting the given status (no-op if bookmark already exists).
 func (p *Processor) BookmarkCreate(ctx context.Context, requestingAccount *gtsmodel.Account, targetStatusID string) (*apimodel.Status, gtserror.WithCode) {
-	targetStatus, existingBookmarkID, errWithCode := p.getBookmarkableStatus(ctx, requestingAccount, targetStatusID)
+	targetStatus, existing, errWithCode := p.getBookmarkableStatus(ctx, requestingAccount, targetStatusID)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}
 
-	if existingBookmarkID != "" {
+	if existing != nil {
 		// Status is already bookmarked.
 		return p.c.GetAPIStatus(ctx, requestingAccount, targetStatus)
 	}
@@ -86,18 +67,18 @@ func (p *Processor) BookmarkCreate(ctx context.Context, requestingAccount *gtsmo
 
 // BookmarkRemove removes a bookmark for the requesting account, targeting the given status (no-op if bookmark doesn't exist).
 func (p *Processor) BookmarkRemove(ctx context.Context, requestingAccount *gtsmodel.Account, targetStatusID string) (*apimodel.Status, gtserror.WithCode) {
-	targetStatus, existingBookmarkID, errWithCode := p.getBookmarkableStatus(ctx, requestingAccount, targetStatusID)
+	targetStatus, existing, errWithCode := p.getBookmarkableStatus(ctx, requestingAccount, targetStatusID)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}
 
-	if existingBookmarkID == "" {
+	if existing == nil {
 		// Status isn't bookmarked.
 		return p.c.GetAPIStatus(ctx, requestingAccount, targetStatus)
 	}
 
 	// We have a bookmark to remove.
-	if err := p.state.DB.DeleteStatusBookmark(ctx, existingBookmarkID); err != nil {
+	if err := p.state.DB.DeleteStatusBookmarkByID(ctx, existing.ID); err != nil {
 		err = gtserror.Newf("error removing status bookmark: %w", err)
 		return nil, gtserror.NewErrorInternalError(err)
 	}
@@ -109,3 +90,34 @@ func (p *Processor) BookmarkRemove(ctx context.Context, requestingAccount *gtsmo
 
 	return p.c.GetAPIStatus(ctx, requestingAccount, targetStatus)
 }
+
+func (p *Processor) getBookmarkableStatus(
+	ctx context.Context,
+	requester *gtsmodel.Account,
+	statusID string,
+) (
+	*gtsmodel.Status,
+	*gtsmodel.StatusBookmark,
+	gtserror.WithCode,
+) {
+	target, errWithCode := p.c.GetVisibleTargetStatus(ctx,
+		requester,
+		statusID,
+		nil, // default freshness
+	)
+	if errWithCode != nil {
+		return nil, nil, errWithCode
+	}
+
+	bookmark, err := p.state.DB.GetStatusBookmark(
+		gtscontext.SetBarebones(ctx),
+		requester.ID,
+		statusID,
+	)
+	if err != nil && !errors.Is(err, db.ErrNoEntries) {
+		err := gtserror.Newf("error getting bookmark: %w", err)
+		return nil, nil, gtserror.NewErrorInternalError(err)
+	}
+
+	return target, bookmark, nil
+}

internal/typeutils/util.go

@@ -65,7 +65,7 @@ func (c *Converter) interactionsWithStatusForAccount(ctx context.Context, s *gts
 		}
 		si.Muted = muted
 
-		bookmarked, err := c.state.DB.IsStatusBookmarkedBy(ctx, s, requestingAccount.ID)
+		bookmarked, err := c.state.DB.IsStatusBookmarkedBy(ctx, requestingAccount.ID, s.ID)
 		if err != nil {
 			return nil, fmt.Errorf("error checking if requesting account has bookmarked status: %s", err)
 		}

test/envparsing.sh

@@ -54,6 +54,8 @@ EXPECT=$(cat << "EOF"
         "poll-vote-ids-mem-ratio": 2,
         "poll-vote-mem-ratio": 2,
         "report-mem-ratio": 1,
+        "status-bookmark-ids-mem-ratio": 2,
+        "status-bookmark-mem-ratio": 0.5,
         "status-fave-ids-mem-ratio": 3,
         "status-fave-mem-ratio": 2,
         "status-mem-ratio": 5,