gtsocial-umbx

Unnamed repository; edit this file 'description' to name the repository.
Log | Files | Refs | README | LICENSE

commit e46323c207230b01e48ae1b8e5117a9774d9f157
parent 3510454768b1877540c6dc25f4967e4b608203a8
Author: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com>
Date:   Thu,  6 Apr 2023 16:11:25 +0100

[bugfix] visibility caching and hometimeline (#1675)

* fix visibility caching to use correct type key

Signed-off-by: kim <grufwub@gmail.com>

* check for ID check > max possible ID

Signed-off-by: kim <grufwub@gmail.com>

* update home timeline code to include relevant threads to owner (e.g. between mutuals/follows)

Signed-off-by: kim <grufwub@gmail.com>

---------

Signed-off-by: kim <grufwub@gmail.com>
Diffstat:
Minternal/db/bundb/timeline.go | 2+-
Minternal/visibility/account.go | 6++++--
Minternal/visibility/home_timeline.go | 108++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
Minternal/visibility/public_timeline.go | 8+++++---
Minternal/visibility/status.go | 12+++++++-----
5 files changed, 110 insertions(+), 26 deletions(-)

diff --git a/internal/db/bundb/timeline.go b/internal/db/bundb/timeline.go @@ -61,7 +61,7 @@ func (t *timelineDB) GetHomeTimeline(ctx context.Context, accountID string, maxI bun.Ident("follow.account_id"), accountID) - if maxID == "" || maxID == id.Highest { + if maxID == "" || maxID >= id.Highest { const future = 24 * time.Hour var err error diff --git a/internal/visibility/account.go b/internal/visibility/account.go @@ -29,6 +29,8 @@ import ( // AccountVisible will check if given account is visible to requester, accounting for requester with no auth (i.e is nil), suspensions, disabled local users and account blocks. func (f *Filter) AccountVisible(ctx context.Context, requester *gtsmodel.Account, account *gtsmodel.Account) (bool, error) { + const vtype = cache.VisibilityTypeAccount + // By default we assume no auth. requesterID := noauth @@ -48,10 +50,10 @@ func (f *Filter) AccountVisible(ctx context.Context, requester *gtsmodel.Account return &cache.CachedVisibility{ ItemID: account.ID, RequesterID: requesterID, - Type: cache.VisibilityTypeAccount, + Type: vtype, Value: visible, }, nil - }, "account", requesterID, account.ID) + }, vtype, requesterID, account.ID) if err != nil { return false, err } diff --git a/internal/visibility/home_timeline.go b/internal/visibility/home_timeline.go @@ -30,6 +30,8 @@ import ( // StatusHomeTimelineable checks if given status should be included on owner's home timeline. Primarily relying on status visibility to owner and the AP visibility setting, but also taking into account thread replies etc. func (f *Filter) StatusHomeTimelineable(ctx context.Context, owner *gtsmodel.Account, status *gtsmodel.Status) (bool, error) { + const vtype = cache.VisibilityTypeHome + // By default we assume no auth. requesterID := noauth @@ -49,10 +51,10 @@ func (f *Filter) StatusHomeTimelineable(ctx context.Context, owner *gtsmodel.Acc return &cache.CachedVisibility{ ItemID: status.ID, RequesterID: requesterID, - Type: cache.VisibilityTypeHome, + Type: vtype, Value: visible, }, nil - }, "home", requesterID, status.ID) + }, vtype, requesterID, status.ID) if err != nil { if err == cache.SentinelError { // Filter-out our temporary @@ -95,21 +97,22 @@ func (f *Filter) isStatusHomeTimelineable(ctx context.Context, owner *gtsmodel.A } var ( - parent *gtsmodel.Status - included bool + next *gtsmodel.Status oneAuthor bool + included bool + converstn bool ) - for parent = status; parent.InReplyToURI != ""; { + for next = status; next.InReplyToURI != ""; { // Fetch next parent to lookup. - parentID := parent.InReplyToID + parentID := next.InReplyToID if parentID == "" { - log.Warnf(ctx, "status not yet deref'd: %s", parent.InReplyToURI) + log.Warnf(ctx, "status not yet deref'd: %s", next.InReplyToURI) return false, cache.SentinelError } // Get the next parent in the chain from DB. - parent, err = f.state.DB.GetStatusByID( + next, err = f.state.DB.GetStatusByID( gtscontext.SetBarebones(ctx), parentID, ) @@ -117,28 +120,49 @@ func (f *Filter) isStatusHomeTimelineable(ctx context.Context, owner *gtsmodel.A return false, fmt.Errorf("isStatusHomeTimelineable: error getting status parent %s: %w", parentID, err) } - if (parent.AccountID == owner.ID) || - parent.MentionsAccount(owner.ID) { + // Populate account mention objects before account mention checks. + next.Mentions, err = f.state.DB.GetMentions(ctx, next.MentionIDs) + if err != nil { + return false, fmt.Errorf("isStatusHomeTimelineable: error populating status parent %s mentions: %w", parentID, err) + } + + if (next.AccountID == owner.ID) || + next.MentionsAccount(owner.ID) { // Owner is in / mentioned in - // this status thread. + // this status thread. They can + // see all future visible statuses. included = true break } + // Check whether this should be a visible conversation, i.e. + // is it between accounts on owner timeline that they follow? + converstn, err = f.isVisibleConversation(ctx, owner, next) + if err != nil { + return false, fmt.Errorf("isStatusHomeTimelineable: error checking conversation visibility: %w", err) + } + + if converstn { + // Owner is relevant to this conversation, + // i.e. between follows / mutuals they know. + break + } + if oneAuthor { // Check if this is a single-author status thread. - oneAuthor = (parent.AccountID == status.AccountID) + oneAuthor = (next.AccountID == status.AccountID) } } - if parent != status && !included && !oneAuthor { - log.Trace(ctx, "ignoring visible reply to conversation thread excluding owner") + if next != status && !oneAuthor && !included && !converstn { + log.Trace(ctx, "ignoring visible reply in conversation irrelevant to owner") return false, nil } // At this point status is either a top-level status, a reply in a single // author thread (e.g. "this is my weird-ass take and here is why 1/10 🧵"), - // or a thread mentioning / including timeline owner. + // a status thread *including* the owner, or a conversation thread between + // accounts the timeline owner follows. if status.Visibility == gtsmodel.VisibilityFollowersOnly || status.Visibility == gtsmodel.VisibilityMutualsOnly { @@ -163,3 +187,57 @@ func (f *Filter) isStatusHomeTimelineable(ctx context.Context, owner *gtsmodel.A return true, nil } + +func (f *Filter) isVisibleConversation(ctx context.Context, owner *gtsmodel.Account, status *gtsmodel.Status) (bool, error) { + // Check if status is visible to the timeline owner. + visible, err := f.StatusVisible(ctx, owner, status) + if err != nil { + return false, err + } + + if !visible { + // Invisible to + // timeline owner. + return false, nil + } + + if status.Visibility == gtsmodel.VisibilityUnlocked || + status.Visibility == gtsmodel.VisibilityPublic { + // NOTE: there is no need to check in the case of + // direct / follow-only / mutual-only visibility statuses + // as the above visibility check already handles this. + + // Check if owner follows the status author. + followAuthor, err := f.state.DB.IsFollowing(ctx, + owner.ID, + status.AccountID, + ) + if err != nil { + return false, fmt.Errorf("error checking follow %s->%s: %w", owner.ID, status.AccountID, err) + } + + if !followAuthor { + // Not a visible status + // in conversation thread. + return false, nil + } + } + + for _, mention := range status.Mentions { + // Check if timeline owner follows target. + follow, err := f.state.DB.IsFollowing(ctx, + owner.ID, + mention.TargetAccountID, + ) + if err != nil { + return false, fmt.Errorf("error checking mention follow %s->%s: %w", owner.ID, mention.TargetAccountID, err) + } + + if follow { + // Confirmed conversation. + return true, nil + } + } + + return false, nil +} diff --git a/internal/visibility/public_timeline.go b/internal/visibility/public_timeline.go @@ -30,6 +30,8 @@ import ( // StatusHomeTimelineable checks if given status should be included on requester's public timeline. Primarily relying on status visibility to requester and the AP visibility setting, and ignoring conversation threads. func (f *Filter) StatusPublicTimelineable(ctx context.Context, requester *gtsmodel.Account, status *gtsmodel.Status) (bool, error) { + const vtype = cache.VisibilityTypePublic + // By default we assume no auth. requesterID := noauth @@ -49,10 +51,10 @@ func (f *Filter) StatusPublicTimelineable(ctx context.Context, requester *gtsmod return &cache.CachedVisibility{ ItemID: status.ID, RequesterID: requesterID, - Type: cache.VisibilityTypePublic, + Type: vtype, Value: visible, }, nil - }, "public", requesterID, status.ID) + }, vtype, requesterID, status.ID) if err != nil { if err == cache.SentinelError { // Filter-out our temporary @@ -103,7 +105,7 @@ func (f *Filter) isStatusPublicTimelineable(ctx context.Context, requester *gtsm parentID, ) if err != nil { - return false, fmt.Errorf("isStatusHomeTimelineable: error getting status parent %s: %w", parentID, err) + return false, fmt.Errorf("isStatusPublicTimelineable: error getting status parent %s: %w", parentID, err) } if parent.AccountID != status.AccountID { diff --git a/internal/visibility/status.go b/internal/visibility/status.go @@ -49,6 +49,8 @@ func (f *Filter) StatusesVisible(ctx context.Context, requester *gtsmodel.Accoun // StatusVisible will check if given status is visible to requester, accounting for requester with no auth (i.e is nil), suspensions, disabled local users, account blocks and status privacy. func (f *Filter) StatusVisible(ctx context.Context, requester *gtsmodel.Account, status *gtsmodel.Status) (bool, error) { + const vtype = cache.VisibilityTypeStatus + // By default we assume no auth. requesterID := noauth @@ -68,10 +70,10 @@ func (f *Filter) StatusVisible(ctx context.Context, requester *gtsmodel.Account, return &cache.CachedVisibility{ ItemID: status.ID, RequesterID: requesterID, - Type: cache.VisibilityTypeStatus, + Type: vtype, Value: visible, }, nil - }, "status", requesterID, status.ID) + }, vtype, requesterID, status.ID) if err != nil { return false, err } @@ -83,7 +85,7 @@ func (f *Filter) StatusVisible(ctx context.Context, requester *gtsmodel.Account, func (f *Filter) isStatusVisible(ctx context.Context, requester *gtsmodel.Account, status *gtsmodel.Status) (bool, error) { // Ensure that status is fully populated for further processing. if err := f.state.DB.PopulateStatus(ctx, status); err != nil { - return false, err + return false, fmt.Errorf("isStatusVisible: error populating status %s: %w", status.ID, err) } // Check whether status accounts are visible to the requester. @@ -185,7 +187,7 @@ func (f *Filter) areStatusAccountsVisible(ctx context.Context, requester *gtsmod // Check whether status author's account is visible to requester. visible, err := f.AccountVisible(ctx, requester, status.Account) if err != nil { - return false, err + return false, fmt.Errorf("error checking status author visibility: %w", err) } if !visible { @@ -204,7 +206,7 @@ func (f *Filter) areStatusAccountsVisible(ctx context.Context, requester *gtsmod // Check whether boosted status author's account is visible to requester. visible, err := f.AccountVisible(ctx, requester, status.BoostOfAccount) if err != nil { - return false, err + return false, fmt.Errorf("error checking boosted author visibility: %w", err) } if !visible {