diff options
| author | 2024-09-16 16:46:09 +0000 | |
|---|---|---|
| committer | 2024-09-16 16:46:09 +0000 | |
| commit | 84279f6a6a0201c90a6747fe8b82c38d5b4e49e2 (patch) | |
| tree | 6c777c7ed4888d990533117d7e63376bcc23a3fb /internal/processing | |
| parent | [chore] Refactor federatingDB.Undo, avoid 500 errors on Undo Like (#3310) (diff) | |
| download | gotosocial-84279f6a6a0201c90a6747fe8b82c38d5b4e49e2.tar.xz | |
[performance] cache more database calls, reduce required database calls overall (#3290)
* improvements to caching for lists and relationship to accounts / follows
* fix nil panic in AddToList()
* ensure list related caches are correctly invalidated
* ensure returned ID lists are ordered correctly
* bump go-structr to v0.8.9 (returns early if zero uncached keys to be loaded)
* remove zero checks in uncached key load functions (go-structr now handles this)
* fix issues after rebase on upstream/main
* update the expected return order of CSV exports (since list entries are now down by entry creation date)
* rename some funcs, allow deleting list entries for multiple follow IDs at a time, fix up more tests
* use returning statements on delete to get cache invalidation info
* fixes to recent database delete changes
* fix broken list entries delete sql
* remove unused db function
* update remainder of delete functions to behave in similar way, some other small tweaks
* fix delete user sql, allow returning on err no entries
* uncomment + fix list database tests
* update remaining list tests
* update envparsing test
* add comments to each specific key being invalidated
* add more cache invalidation explanatory comments
* whoops; actually delete poll votes from database in the DeletePollByID() func
* remove added but-commented-out field
* improved comment regarding paging being disabled
* make cache invalidation comments match what's actually happening
* fix up delete query comments to match what is happening
* rename function to read a bit better
* don't use ErrNoEntries on delete when not needed (it's only needed for a RETURNING call)
* update function name in test
* move list exclusivity check to AFTER eligibility check. use log.Panic() instead of panic()
* use the poll_id column in poll_votes for selecting votes in poll ID
* fix function name
Diffstat (limited to 'internal/processing')
| -rw-r--r-- | internal/processing/account/export.go | 2 | ||||
| -rw-r--r-- | internal/processing/account/lists.go | 43 | ||||
| -rw-r--r-- | internal/processing/admin/report.go | 4 | ||||
| -rw-r--r-- | internal/processing/common/status.go | 2 | ||||
| -rw-r--r-- | internal/processing/list/get.go | 138 | ||||
| -rw-r--r-- | internal/processing/list/updateentries.go | 177 | ||||
| -rw-r--r-- | internal/processing/list/util.go | 46 | ||||
| -rw-r--r-- | internal/processing/workers/fromclientapi_test.go | 3 | ||||
| -rw-r--r-- | internal/processing/workers/surfacetimeline.go | 343 | ||||
| -rw-r--r-- | internal/processing/workers/util.go | 5 | 
10 files changed, 313 insertions, 450 deletions
diff --git a/internal/processing/account/export.go b/internal/processing/account/export.go index 9954ea225..68cc17b6d 100644 --- a/internal/processing/account/export.go +++ b/internal/processing/account/export.go @@ -98,7 +98,7 @@ func (p *Processor) ExportLists(  	ctx context.Context,  	requester *gtsmodel.Account,  ) ([][]string, gtserror.WithCode) { -	lists, err := p.state.DB.GetListsForAccountID(ctx, requester.ID) +	lists, err := p.state.DB.GetListsByAccountID(ctx, requester.ID)  	if err != nil && !errors.Is(err, db.ErrNoEntries) {  		err = gtserror.Newf("db error getting lists: %w", err)  		return nil, gtserror.NewErrorInternalError(err) diff --git a/internal/processing/account/lists.go b/internal/processing/account/lists.go index 1d92bee82..04cf4ca73 100644 --- a/internal/processing/account/lists.go +++ b/internal/processing/account/lists.go @@ -30,8 +30,6 @@ import (  	"github.com/superseriousbusiness/gotosocial/internal/log"  ) -var noLists = make([]*apimodel.List, 0) -  // ListsGet returns all lists owned by requestingAccount, which contain a follow for targetAccountID.  func (p *Processor) ListsGet(ctx context.Context, requestingAccount *gtsmodel.Account, targetAccountID string) ([]*apimodel.List, gtserror.WithCode) {  	targetAccount, err := p.state.DB.GetAccountByID(ctx, targetAccountID) @@ -54,52 +52,35 @@ func (p *Processor) ListsGet(ctx context.Context, requestingAccount *gtsmodel.Ac  	// Requester has to follow targetAccount  	// for them to be in any of their lists.  	follow, err := p.state.DB.GetFollow( +  		// Don't populate follow.  		gtscontext.SetBarebones(ctx),  		requestingAccount.ID,  		targetAccountID,  	)  	if err != nil && !errors.Is(err, db.ErrNoEntries) { -		return nil, gtserror.NewErrorInternalError(fmt.Errorf("db error: %w", err)) +		err := gtserror.Newf("error getting follow: %w", err) +		return nil, gtserror.NewErrorInternalError(err)  	}  	if follow == nil { -		return noLists, nil // by definition we know they're in no lists -	} - -	listEntries, err := p.state.DB.GetListEntriesForFollowID( -		// Don't populate entries. -		gtscontext.SetBarebones(ctx), -		follow.ID, -	) -	if err != nil && !errors.Is(err, db.ErrNoEntries) { -		return nil, gtserror.NewErrorInternalError(fmt.Errorf("db error: %w", err)) +		return []*apimodel.List{}, nil  	} -	count := len(listEntries) -	if count == 0 { -		return noLists, nil +	// Get all lists that this follow is an entry within. +	lists, err := p.state.DB.GetListsContainingFollowID(ctx, follow.ID) +	if err != nil { +		err := gtserror.Newf("error getting lists for follow: %w", err) +		return nil, gtserror.NewErrorInternalError(err)  	} -	apiLists := make([]*apimodel.List, 0, count) -	for _, listEntry := range listEntries { -		list, err := p.state.DB.GetListByID( -			// Don't populate list. -			gtscontext.SetBarebones(ctx), -			listEntry.ListID, -		) - -		if err != nil { -			log.Debugf(ctx, "skipping list %s due to error %q", listEntry.ListID, err) -			continue -		} - +	apiLists := make([]*apimodel.List, 0, len(lists)) +	for _, list := range lists {  		apiList, err := p.converter.ListToAPIList(ctx, list)  		if err != nil { -			log.Debugf(ctx, "skipping list %s due to error %q", listEntry.ListID, err) +			log.Errorf(ctx, "error converting list: %v", err)  			continue  		} -  		apiLists = append(apiLists, apiList)  	} diff --git a/internal/processing/admin/report.go b/internal/processing/admin/report.go index 13b5a9d86..ed34a4e83 100644 --- a/internal/processing/admin/report.go +++ b/internal/processing/admin/report.go @@ -142,7 +142,7 @@ func (p *Processor) ReportResolve(ctx context.Context, account *gtsmodel.Account  		columns = append(columns, "action_taken")  	} -	updatedReport, err := p.state.DB.UpdateReport(ctx, report, columns...) +	err = p.state.DB.UpdateReport(ctx, report, columns...)  	if err != nil {  		return nil, gtserror.NewErrorInternalError(err)  	} @@ -156,7 +156,7 @@ func (p *Processor) ReportResolve(ctx context.Context, account *gtsmodel.Account  		Target:         report.Account,  	}) -	apimodelReport, err := p.converter.ReportToAdminAPIReport(ctx, updatedReport, account) +	apimodelReport, err := p.converter.ReportToAdminAPIReport(ctx, report, account)  	if err != nil {  		return nil, gtserror.NewErrorInternalError(err)  	} diff --git a/internal/processing/common/status.go b/internal/processing/common/status.go index 3ef643292..a1d432eb0 100644 --- a/internal/processing/common/status.go +++ b/internal/processing/common/status.go @@ -189,7 +189,7 @@ func (p *Processor) GetAPIStatus(  // such invalidation will, in that case, be handled by the processor instead.  func (p *Processor) InvalidateTimelinedStatus(ctx context.Context, accountID string, statusID string) error {  	// Get lists first + bail if this fails. -	lists, err := p.state.DB.GetListsForAccountID(ctx, accountID) +	lists, err := p.state.DB.GetListsByAccountID(ctx, accountID)  	if err != nil {  		return gtserror.Newf("db error getting lists for account %s: %w", accountID, err)  	} diff --git a/internal/processing/list/get.go b/internal/processing/list/get.go index cdd3c6e0c..b98678eef 100644 --- a/internal/processing/list/get.go +++ b/internal/processing/list/get.go @@ -20,7 +20,6 @@ package list  import (  	"context"  	"errors" -	"fmt"  	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"  	"github.com/superseriousbusiness/gotosocial/internal/db" @@ -28,7 +27,7 @@ import (  	"github.com/superseriousbusiness/gotosocial/internal/gtserror"  	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"  	"github.com/superseriousbusiness/gotosocial/internal/log" -	"github.com/superseriousbusiness/gotosocial/internal/util" +	"github.com/superseriousbusiness/gotosocial/internal/paging"  )  // Get returns the api model of one list with the given ID. @@ -49,16 +48,14 @@ func (p *Processor) Get(ctx context.Context, account *gtsmodel.Account, id strin  // GetAll returns multiple lists created by the given account, sorted by list ID DESC (newest first).  func (p *Processor) GetAll(ctx context.Context, account *gtsmodel.Account) ([]*apimodel.List, gtserror.WithCode) { -	lists, err := p.state.DB.GetListsForAccountID( +	lists, err := p.state.DB.GetListsByAccountID( +  		// Use barebones ctx; no embedded  		// structs necessary for simple GET.  		gtscontext.SetBarebones(ctx),  		account.ID,  	) -	if err != nil { -		if errors.Is(err, db.ErrNoEntries) { -			return nil, nil -		} +	if err != nil && !errors.Is(err, db.ErrNoEntries) {  		return nil, gtserror.NewErrorInternalError(err)  	} @@ -68,66 +65,23 @@ func (p *Processor) GetAll(ctx context.Context, account *gtsmodel.Account) ([]*a  		if errWithCode != nil {  			return nil, errWithCode  		} -  		apiLists = append(apiLists, apiList)  	}  	return apiLists, nil  } -// GetAllListAccounts returns all accounts that are in the given list, -// owned by the given account. There's no pagination for this endpoint. -// -// See https://docs.joinmastodon.org/methods/lists/#query-parameters: -// -//	Limit: Integer. Maximum number of results. Defaults to 40 accounts. -//	Max 80 accounts. Set to 0 in order to get all accounts without pagination. -func (p *Processor) GetAllListAccounts( -	ctx context.Context, -	account *gtsmodel.Account, -	listID string, -) ([]*apimodel.Account, gtserror.WithCode) { -	// Ensure list exists + is owned by requesting account. -	_, errWithCode := p.getList( -		// Use barebones ctx; no embedded -		// structs necessary for this call. -		gtscontext.SetBarebones(ctx), -		account.ID, -		listID, -	) -	if errWithCode != nil { -		return nil, errWithCode -	} - -	// Get all entries for this list. -	listEntries, err := p.state.DB.GetListEntries(ctx, listID, "", "", "", 0) -	if err != nil && !errors.Is(err, db.ErrNoEntries) { -		err = gtserror.Newf("error getting list entries: %w", err) -		return nil, gtserror.NewErrorInternalError(err) -	} - -	// Extract accounts from list entries + add them to response. -	accounts := make([]*apimodel.Account, 0, len(listEntries)) -	p.accountsFromListEntries(ctx, listEntries, func(acc *apimodel.Account) { -		accounts = append(accounts, acc) -	}) - -	return accounts, nil -} -  // GetListAccounts returns accounts that are in the given list, owned by the given account. -// The additional parameters can be used for paging. +// The additional parameters can be used for paging. Nil page param returns all accounts.  func (p *Processor) GetListAccounts(  	ctx context.Context,  	account *gtsmodel.Account,  	listID string, -	maxID string, -	sinceID string, -	minID string, -	limit int, +	page *paging.Page,  ) (*apimodel.PageableResponse, gtserror.WithCode) {  	// Ensure list exists + is owned by requesting account.  	_, errWithCode := p.getList( +  		// Use barebones ctx; no embedded  		// structs necessary for this call.  		gtscontext.SetBarebones(ctx), @@ -138,71 +92,45 @@ func (p *Processor) GetListAccounts(  		return nil, errWithCode  	} -	// To know which accounts are in the list, -	// we need to first get requested list entries. -	listEntries, err := p.state.DB.GetListEntries(ctx, listID, maxID, sinceID, minID, limit) -	if err != nil && !errors.Is(err, db.ErrNoEntries) { -		err = fmt.Errorf("GetListAccounts: error getting list entries: %w", err) +	// Get all accounts contained within list. +	accounts, err := p.state.DB.GetAccountsInList(ctx, +		listID, +		page, +	) +	if err != nil { +		err := gtserror.Newf("db error getting accounts in list: %w", err)  		return nil, gtserror.NewErrorInternalError(err)  	} -	count := len(listEntries) +	// Check for any accounts. +	count := len(accounts)  	if count == 0 { -		// No list entries means no accounts. -		return util.EmptyPageableResponse(), nil +		return paging.EmptyResponse(), nil  	}  	var ( +		// Preallocate expected frontend items.  		items = make([]interface{}, 0, count) -		// Set next + prev values before filtering and API -		// converting, so caller can still page properly. -		nextMaxIDValue = listEntries[count-1].ID -		prevMinIDValue = listEntries[0].ID +		// Set paging low / high IDs. +		lo = accounts[count-1].ID +		hi = accounts[0].ID  	) -	// Extract accounts from list entries + add them to response. -	p.accountsFromListEntries(ctx, listEntries, func(acc *apimodel.Account) { -		items = append(items, acc) -	}) - -	return util.PackagePageableResponse(util.PageableResponseParams{ -		Items:          items, -		Path:           "/api/v1/lists/" + listID + "/accounts", -		NextMaxIDValue: nextMaxIDValue, -		PrevMinIDValue: prevMinIDValue, -		Limit:          limit, -	}) -} - -func (p *Processor) accountsFromListEntries( -	ctx context.Context, -	listEntries []*gtsmodel.ListEntry, -	appendAcc func(*apimodel.Account), -) { -	// For each list entry, we want the account it points to. -	// To get this, we need to first get the follow that the -	// list entry pertains to, then extract the target account -	// from that follow. -	// -	// We do paging not by account ID, but by list entry ID. -	for _, listEntry := range listEntries { -		if err := p.state.DB.PopulateListEntry(ctx, listEntry); err != nil { -			log.Errorf(ctx, "error populating list entry: %v", err) -			continue -		} - -		if err := p.state.DB.PopulateFollow(ctx, listEntry.Follow); err != nil { -			log.Errorf(ctx, "error populating follow: %v", err) -			continue -		} - -		apiAccount, err := p.converter.AccountToAPIAccountPublic(ctx, listEntry.Follow.TargetAccount) +	// Convert accounts to frontend. +	for _, account := range accounts { +		apiAccount, err := p.converter.AccountToAPIAccountPublic(ctx, account)  		if err != nil { -			log.Errorf(ctx, "error converting to public api account: %v", err) +			log.Errorf(ctx, "error converting to api account: %v", err)  			continue  		} - -		appendAcc(apiAccount) +		items = append(items, apiAccount)  	} + +	return paging.PackageResponse(paging.ResponseParams{ +		Items: items, +		Path:  "/api/v1/lists/" + listID + "/accounts", +		Next:  page.Next(lo, hi), +		Prev:  page.Prev(lo, hi), +	}), nil  } diff --git a/internal/processing/list/updateentries.go b/internal/processing/list/updateentries.go index 6dcb951a7..c15248f39 100644 --- a/internal/processing/list/updateentries.go +++ b/internal/processing/list/updateentries.go @@ -23,73 +23,90 @@ import (  	"fmt"  	"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" +	"github.com/superseriousbusiness/gotosocial/internal/util"  )  // AddToList adds targetAccountIDs to the given list, if valid.  func (p *Processor) AddToList(ctx context.Context, account *gtsmodel.Account, listID string, targetAccountIDs []string) gtserror.WithCode { +  	// Ensure this list exists + account owns it. -	list, errWithCode := p.getList(ctx, account.ID, listID) +	_, errWithCode := p.getList(ctx, account.ID, listID)  	if errWithCode != nil {  		return errWithCode  	} -	// Pre-assemble list of entries to add. We *could* add these -	// one by one as we iterate through accountIDs, but according -	// to the Mastodon API we should only add them all once we know -	// they're all valid, no partial updates. -	listEntries := make([]*gtsmodel.ListEntry, 0, len(targetAccountIDs)) +	// Get all follows that are entries in list. +	follows, err := p.state.DB.GetFollowsInList( + +		// We only need barebones model. +		gtscontext.SetBarebones(ctx), +		listID, +		nil, +	) +	if err != nil { +		err := gtserror.Newf("error getting list follows: %w", err) +		return gtserror.NewErrorInternalError(err) +	} + +	// Convert the follows to a hash set containing the target account IDs. +	inFollows := util.ToSetFunc(follows, func(follow *gtsmodel.Follow) string { +		return follow.TargetAccountID +	}) -	// Check each targetAccountID is valid. -	//   - Follow must exist. -	//   - Follow must not already be in the given list. +	// Preallocate a slice of expected list entries, we specifically +	// gather and add all the target accounts in one go rather than +	// individually, to ensure we don't end up with partial updates. +	entries := make([]*gtsmodel.ListEntry, 0, len(targetAccountIDs)) + +	// Iterate all the account IDs in given target list.  	for _, targetAccountID := range targetAccountIDs { -		// Ensure follow exists. -		follow, err := p.state.DB.GetFollow(ctx, account.ID, targetAccountID) -		if err != nil { -			if errors.Is(err, db.ErrNoEntries) { -				err = fmt.Errorf("you do not follow account %s", targetAccountID) -				return gtserror.NewErrorNotFound(err, err.Error()) -			} -			return gtserror.NewErrorInternalError(err) + +		// Look for follow to target account. +		if inFollows.Has(targetAccountID) { +			text := fmt.Sprintf("account %s is already in list %s", targetAccountID, listID) +			return gtserror.NewErrorUnprocessableEntity(errors.New(text), text)  		} -		// Ensure followID not already in list. -		// This particular call to isInList will -		// never error, so just check entryID. -		entryID, _ := isInList( -			list, -			follow.ID, -			func(listEntry *gtsmodel.ListEntry) (string, error) { -				// Looking for the listEntry follow ID. -				return listEntry.FollowID, nil -			}, +		// Get the actual follow to target. +		follow, err := p.state.DB.GetFollow( + +			// We don't need any sub-models. +			gtscontext.SetBarebones(ctx), +			account.ID, +			targetAccountID,  		) +		if err != nil && !errors.Is(err, db.ErrNoEntries) { +			err := gtserror.Newf("db error getting follow: %w", err) +			return gtserror.NewErrorInternalError(err) +		} -		// Empty entryID means entry with given -		// followID wasn't found in the list. -		if entryID != "" { -			err = fmt.Errorf("account with id %s is already in list %s with entryID %s", targetAccountID, listID, entryID) -			return gtserror.NewErrorUnprocessableEntity(err, err.Error()) +		if follow == nil { +			text := fmt.Sprintf("account %s not currently followed", targetAccountID) +			return gtserror.NewErrorNotFound(errors.New(text), text)  		} -		// Entry wasn't in the list, we can add it. -		listEntries = append(listEntries, >smodel.ListEntry{ +		// Generate new entry for this follow in list. +		entries = append(entries, >smodel.ListEntry{  			ID:       id.NewULID(),  			ListID:   listID,  			FollowID: follow.ID,  		})  	} -	// If we get to here we can assume all -	// entries are valid, so try to add them. -	if err := p.state.DB.PutListEntries(ctx, listEntries); err != nil { -		if errors.Is(err, db.ErrAlreadyExists) { -			err = fmt.Errorf("one or more errors inserting list entries: %w", err) -			return gtserror.NewErrorUnprocessableEntity(err, err.Error()) -		} +	// Add all of the gathered list entries to the database. +	switch err := p.state.DB.PutListEntries(ctx, entries); { +	case err == nil: + +	case errors.Is(err, db.ErrAlreadyExists): +		err := gtserror.Newf("conflict adding list entry: %w", err) +		return gtserror.NewErrorUnprocessableEntity(err) + +	default: +		err := gtserror.Newf("db error inserting list entries: %w", err)  		return gtserror.NewErrorInternalError(err)  	} @@ -97,55 +114,61 @@ func (p *Processor) AddToList(ctx context.Context, account *gtsmodel.Account, li  }  // RemoveFromList removes targetAccountIDs from the given list, if valid. -func (p *Processor) RemoveFromList(ctx context.Context, account *gtsmodel.Account, listID string, targetAccountIDs []string) gtserror.WithCode { +func (p *Processor) RemoveFromList( +	ctx context.Context, +	account *gtsmodel.Account, +	listID string, +	targetAccountIDs []string, +) gtserror.WithCode {  	// Ensure this list exists + account owns it. -	list, errWithCode := p.getList(ctx, account.ID, listID) +	_, errWithCode := p.getList(ctx, account.ID, listID)  	if errWithCode != nil {  		return errWithCode  	} -	// For each targetAccountID, we want to check if -	// a follow with that targetAccountID is in the -	// given list. If it is in there, we want to remove -	// it from the list. +	// Get all follows that are entries in list. +	follows, err := p.state.DB.GetFollowsInList( + +		// We only need barebones model. +		gtscontext.SetBarebones(ctx), +		listID, +		nil, +	) +	if err != nil { +		err := gtserror.Newf("error getting list follows: %w", err) +		return gtserror.NewErrorInternalError(err) +	} + +	// Convert the follows to a map keyed by the target account ID. +	followsMap := util.KeyBy(follows, func(follow *gtsmodel.Follow) string { +		return follow.TargetAccountID +	}) + +	var errs gtserror.MultiError + +	// Iterate all the account IDs in given target list.  	for _, targetAccountID := range targetAccountIDs { -		// Check if targetAccountID is -		// on a follow in the list. -		entryID, err := isInList( -			list, -			targetAccountID, -			func(listEntry *gtsmodel.ListEntry) (string, error) { -				// We need the follow so populate this -				// entry, if it's not already populated. -				if err := p.state.DB.PopulateListEntry(ctx, listEntry); err != nil { -					return "", err -				} - -				// Looking for the list entry targetAccountID. -				return listEntry.Follow.TargetAccountID, nil -			}, -		) -		// Error may be returned here if there was an issue -		// populating the list entry. We only return on proper -		// DB errors, we can just skip no entry errors. -		if err != nil && !errors.Is(err, db.ErrNoEntries) { -			err = fmt.Errorf("error checking if targetAccountID %s was in list %s: %w", targetAccountID, listID, err) -			return gtserror.NewErrorInternalError(err) -		} +		// Look for follow targetting this account. +		follow, ok := followsMap[targetAccountID] -		if entryID == "" { -			// There was an errNoEntries or targetAccount -			// wasn't in this list anyway, so we can skip it. +		if !ok { +			// not in list.  			continue  		} -		// TargetAccount was in the list, remove the entry. -		if err := p.state.DB.DeleteListEntry(ctx, entryID); err != nil && !errors.Is(err, db.ErrNoEntries) { -			err = fmt.Errorf("error removing list entry %s from list %s: %w", entryID, listID, err) -			return gtserror.NewErrorInternalError(err) +		// Delete the list entry containing follow ID in list. +		err := p.state.DB.DeleteListEntry(ctx, listID, follow.ID) +		if err != nil && !errors.Is(err, db.ErrNoEntries) { +			errs.Appendf("error removing list entry: %w", err) +			continue  		}  	} +	// Wrap errors in errWithCode if set. +	if err := errs.Combine(); err != nil { +		return gtserror.NewErrorInternalError(err) +	} +  	return nil  } diff --git a/internal/processing/list/util.go b/internal/processing/list/util.go index c5b1e5081..74d148704 100644 --- a/internal/processing/list/util.go +++ b/internal/processing/list/util.go @@ -33,18 +33,25 @@ import (  // appropriate errors so caller doesn't need to bother.  func (p *Processor) getList(ctx context.Context, accountID string, listID string) (*gtsmodel.List, gtserror.WithCode) {  	list, err := p.state.DB.GetListByID(ctx, listID) -	if err != nil { -		if errors.Is(err, db.ErrNoEntries) { -			// List doesn't seem to exist. -			return nil, gtserror.NewErrorNotFound(err) -		} -		// Real database error. +	if err != nil && !errors.Is(err, db.ErrNoEntries) { +		err := gtserror.Newf("db error getting list: %w", err)  		return nil, gtserror.NewErrorInternalError(err)  	} +	if list == nil { +		const text = "list not found" +		return nil, gtserror.NewErrorNotFound( +			errors.New(text), +			text, +		) +	} +  	if list.AccountID != accountID { -		err = fmt.Errorf("list with id %s does not belong to account %s", list.ID, accountID) -		return nil, gtserror.NewErrorNotFound(err) +		const text = "list not found" +		return nil, gtserror.NewErrorNotFound( +			errors.New("list does not belong to account"), +			text, +		)  	}  	return list, nil @@ -60,26 +67,3 @@ func (p *Processor) apiList(ctx context.Context, list *gtsmodel.List) (*apimodel  	return apiList, nil  } - -// isInList check if thisID is equal to the result of thatID -// for any entry in the given list. -// -// Will return the id of the listEntry if true, empty if false, -// or an error if the result of thatID returns an error. -func isInList( -	list *gtsmodel.List, -	thisID string, -	getThatID func(listEntry *gtsmodel.ListEntry) (string, error), -) (string, error) { -	for _, listEntry := range list.ListEntries { -		thatID, err := getThatID(listEntry) -		if err != nil { -			return "", err -		} - -		if thisID == thatID { -			return listEntry.ID, nil -		} -	} -	return "", nil -} diff --git a/internal/processing/workers/fromclientapi_test.go b/internal/processing/workers/fromclientapi_test.go index cc8801e1c..d955f0529 100644 --- a/internal/processing/workers/fromclientapi_test.go +++ b/internal/processing/workers/fromclientapi_test.go @@ -649,7 +649,8 @@ func (suite *FromClientAPITestSuite) TestProcessCreateStatusListRepliesPolicyLis  	}  	// Remove turtle from the list. -	if err := testStructs.State.DB.DeleteListEntry(ctx, suite.testListEntries["local_account_1_list_1_entry_1"].ID); err != nil { +	testEntry := suite.testListEntries["local_account_1_list_1_entry_1"] +	if err := testStructs.State.DB.DeleteListEntry(ctx, testEntry.ListID, testEntry.FollowID); err != nil {  		suite.FailNow(err.Error())  	} diff --git a/internal/processing/workers/surfacetimeline.go b/internal/processing/workers/surfacetimeline.go index 81544d928..90cb1fed3 100644 --- a/internal/processing/workers/surfacetimeline.go +++ b/internal/processing/workers/surfacetimeline.go @@ -21,7 +21,6 @@ import (  	"context"  	"errors" -	"github.com/superseriousbusiness/gotosocial/internal/db"  	statusfilter "github.com/superseriousbusiness/gotosocial/internal/filter/status"  	"github.com/superseriousbusiness/gotosocial/internal/filter/usermute"  	"github.com/superseriousbusiness/gotosocial/internal/gtscontext" @@ -63,13 +62,9 @@ func (s *Surface) timelineAndNotifyStatus(ctx context.Context, status *gtsmodel.  		})  	} -	// Timeline the status for each local follower of this account. -	// This will also handle notifying any followers with notify -	// set to true on their follow. -	homeTimelinedAccountIDs, err := s.timelineAndNotifyStatusForFollowers(ctx, status, follows) -	if err != nil { -		return gtserror.Newf("error timelining status %s for followers: %w", status.ID, err) -	} +	// Timeline the status for each local follower of this account. This will +	// also handle notifying any followers with notify set to true on their follow. +	homeTimelinedAccountIDs := s.timelineAndNotifyStatusForFollowers(ctx, status, follows)  	// Timeline the status for each local account who follows a tag used by this status.  	if err := s.timelineAndNotifyStatusForTagFollowers(ctx, status, homeTimelinedAccountIDs); err != nil { @@ -105,12 +100,10 @@ func (s *Surface) timelineAndNotifyStatusForFollowers(  	ctx context.Context,  	status *gtsmodel.Status,  	follows []*gtsmodel.Follow, -) ([]string, error) { +) (homeTimelinedAccountIDs []string) {  	var ( -		errs                    gtserror.MultiError -		boost                   = status.BoostOfID != "" -		reply                   = status.InReplyToURI != "" -		homeTimelinedAccountIDs = []string{} +		boost = (status.BoostOfID != "") +		reply = (status.InReplyToURI != "")  	)  	for _, follow := range follows { @@ -130,7 +123,7 @@ func (s *Surface) timelineAndNotifyStatusForFollowers(  			ctx, follow.Account, status,  		)  		if err != nil { -			errs.Appendf("error checking status %s hometimelineability: %w", status.ID, err) +			log.Errorf(ctx, "error checking status home visibility for follow: %v", err)  			continue  		} @@ -139,29 +132,36 @@ func (s *Surface) timelineAndNotifyStatusForFollowers(  			continue  		} +		// Get relevant filters and mutes for this follow's account. +		// (note the origin account of the follow is receiver of status).  		filters, mutes, err := s.getFiltersAndMutes(ctx, follow.AccountID)  		if err != nil { -			errs.Append(err) +			log.Error(ctx, err)  			continue  		} -		// Add status to any relevant lists -		// for this follow, if applicable. -		exclusive, listTimelined := s.listTimelineStatusForFollow( -			ctx, +		// Add status to any relevant lists for this follow, if applicable. +		listTimelined, exclusive, err := s.listTimelineStatusForFollow(ctx,  			status,  			follow, -			&errs,  			filters,  			mutes,  		) +		if err != nil { +			log.Errorf(ctx, "error list timelining status: %v", err) +			continue +		} -		// Add status to home timeline for owner -		// of this follow, if applicable. -		homeTimelined := false +		var homeTimelined bool + +		// If this was timelined into +		// list with exclusive flag set, +		// don't add to home timeline.  		if !exclusive { -			homeTimelined, err = s.timelineStatus( -				ctx, + +			// Add status to home timeline for owner of +			// this follow (origin account), if applicable. +			homeTimelined, err = s.timelineStatus(ctx,  				s.State.Timelines.Home.IngestOne,  				follow.AccountID, // home timelines are keyed by account ID  				follow.Account, @@ -171,10 +171,12 @@ func (s *Surface) timelineAndNotifyStatusForFollowers(  				mutes,  			)  			if err != nil { -				errs.Appendf("error home timelining status: %w", err) +				log.Errorf(ctx, "error home timelining status: %v", err)  				continue  			} +  			if homeTimelined { +				// If hometimelined, add to list of returned account IDs.  				homeTimelinedAccountIDs = append(homeTimelinedAccountIDs, follow.AccountID)  			}  		} @@ -210,11 +212,12 @@ func (s *Surface) timelineAndNotifyStatusForFollowers(  			status.Account,  			status.ID,  		); err != nil { -			errs.Appendf("error notifying account %s about new status: %w", follow.AccountID, err) +			log.Errorf(ctx, "error notifying status for account: %v", err) +			continue  		}  	} -	return homeTimelinedAccountIDs, errs.Combine() +	return homeTimelinedAccountIDs  }  // listTimelineStatusForFollow puts the given status @@ -227,107 +230,59 @@ func (s *Surface) listTimelineStatusForFollow(  	ctx context.Context,  	status *gtsmodel.Status,  	follow *gtsmodel.Follow, -	errs *gtserror.MultiError,  	filters []*gtsmodel.Filter,  	mutes *usermute.CompiledUserMuteList, -) (bool, bool) { -	// To put this status in appropriate list timelines, -	// we need to get each listEntry that pertains to -	// this follow. Then, we want to iterate through all -	// those list entries, and add the status to the list -	// that the entry belongs to if it meets criteria for -	// inclusion in the list. - -	listEntries, err := s.getListEntries(ctx, follow) -	if err != nil { -		errs.Append(err) -		return false, false -	} -	exclusive, err := s.isAnyListExclusive(ctx, listEntries) +) (timelined bool, exclusive bool, err error) { + +	// Get all lists that contain this given follow. +	lists, err := s.State.DB.GetListsContainingFollowID( + +		// We don't need list sub-models. +		gtscontext.SetBarebones(ctx), +		follow.ID, +	)  	if err != nil { -		errs.Append(err) -		return false, false +		return false, false, gtserror.Newf("error getting lists for follow: %w", err)  	} -	// Check eligibility for each list entry (if any). -	listTimelined := false -	for _, listEntry := range listEntries { -		eligible, err := s.listEligible(ctx, listEntry, status) +	for _, list := range lists { +		// Check whether list is eligible for this status. +		eligible, err := s.listEligible(ctx, list, status)  		if err != nil { -			errs.Appendf("error checking list eligibility: %w", err) +			log.Errorf(ctx, "error checking list eligibility: %v", err)  			continue  		}  		if !eligible { -			// Don't add this.  			continue  		} +		// Update exclusive flag if list is so. +		exclusive = exclusive || *list.Exclusive +  		// At this point we are certain this status  		// should be included in the timeline of the  		// list that this list entry belongs to. -		timelined, err := s.timelineStatus( +		listTimelined, err := s.timelineStatus(  			ctx,  			s.State.Timelines.List.IngestOne, -			listEntry.ListID, // list timelines are keyed by list ID +			list.ID, // list timelines are keyed by list ID  			follow.Account,  			status, -			stream.TimelineList+":"+listEntry.ListID, // key streamType to this specific list +			stream.TimelineList+":"+list.ID, // key streamType to this specific list  			filters,  			mutes,  		)  		if err != nil { -			errs.Appendf("error adding status to timeline for list %s: %w", listEntry.ListID, err) -			// implicit continue +			log.Errorf(ctx, "error adding status to list timeline: %v", err) +			continue  		} -		listTimelined = listTimelined || timelined -	} - -	return exclusive, listTimelined -} -// getListEntries returns list entries for a given follow. -func (s *Surface) getListEntries(ctx context.Context, follow *gtsmodel.Follow) ([]*gtsmodel.ListEntry, error) { -	// Get every list entry that targets this follow's ID. -	listEntries, err := s.State.DB.GetListEntriesForFollowID( -		// We only need the list IDs. -		gtscontext.SetBarebones(ctx), -		follow.ID, -	) -	if err != nil && !errors.Is(err, db.ErrNoEntries) { -		return nil, gtserror.Newf("DB error getting list entries: %v", err) +		// Update flag based on if timelined. +		timelined = timelined || listTimelined  	} -	return listEntries, nil -} -// isAnyListExclusive determines whether any provided list entry corresponds to an exclusive list. -func (s *Surface) isAnyListExclusive(ctx context.Context, listEntries []*gtsmodel.ListEntry) (bool, error) { -	if len(listEntries) == 0 { -		return false, nil -	} - -	listIDs := make([]string, 0, len(listEntries)) -	for _, listEntry := range listEntries { -		listIDs = append(listIDs, listEntry.ListID) -	} -	lists, err := s.State.DB.GetListsByIDs( -		// We only need the list exclusive flags. -		gtscontext.SetBarebones(ctx), -		listIDs, -	) -	if err != nil && !errors.Is(err, db.ErrNoEntries) { -		return false, gtserror.Newf("DB error getting lists for list entries: %v", err) -	} - -	if len(lists) == 0 { -		return false, nil -	} -	for _, list := range lists { -		if *list.Exclusive { -			return true, nil -		} -	} -	return false, nil +	return timelined, exclusive, nil  }  // getFiltersAndMutes returns an account's filters and mutes. @@ -341,8 +296,8 @@ func (s *Surface) getFiltersAndMutes(ctx context.Context, accountID string) ([]*  	if err != nil {  		return nil, nil, gtserror.Newf("couldn't retrieve mutes for account %s: %w", accountID, err)  	} -	compiledMutes := usermute.NewCompiledUserMuteList(mutes) +	compiledMutes := usermute.NewCompiledUserMuteList(mutes)  	return filters, compiledMutes, err  } @@ -351,7 +306,7 @@ func (s *Surface) getFiltersAndMutes(ctx context.Context, accountID string) ([]*  // belongs to, based on the replies policy of the list.  func (s *Surface) listEligible(  	ctx context.Context, -	listEntry *gtsmodel.ListEntry, +	list *gtsmodel.List,  	status *gtsmodel.Status,  ) (bool, error) {  	if status.InReplyToURI == "" { @@ -366,18 +321,6 @@ func (s *Surface) listEligible(  		return false, nil  	} -	// Status is a reply to a known account. -	// We need to fetch the list that this -	// entry belongs to, in order to check -	// the list's replies policy. -	list, err := s.State.DB.GetListByID( -		ctx, listEntry.ListID, -	) -	if err != nil { -		err := gtserror.Newf("db error getting list %s: %w", listEntry.ListID, err) -		return false, err -	} -  	switch list.RepliesPolicy {  	case gtsmodel.RepliesPolicyNone:  		// This list should not show @@ -390,20 +333,15 @@ func (s *Surface) listEligible(  		//  		// Check if replied-to account is  		// also included in this list. -		includes, err := s.State.DB.ListIncludesAccount( -			ctx, +		in, err := s.State.DB.IsAccountInList(ctx,  			list.ID,  			status.InReplyToAccountID,  		)  		if err != nil { -			err := gtserror.Newf( -				"db error checking if account %s in list %s: %w", -				status.InReplyToAccountID, listEntry.ListID, err, -			) +			err := gtserror.Newf("db error checking if account in list: %w", err)  			return false, err  		} - -		return includes, nil +		return in, nil  	case gtsmodel.RepliesPolicyFollowed:  		// This list should show replies @@ -418,22 +356,14 @@ func (s *Surface) listEligible(  			status.InReplyToAccountID,  		)  		if err != nil { -			err := gtserror.Newf( -				"db error checking if account %s is followed by %s: %w", -				status.InReplyToAccountID, list.AccountID, err, -			) +			err := gtserror.Newf("db error checking if account followed: %w", err)  			return false, err  		} -  		return follows, nil  	default: -		// HUH?? -		err := gtserror.Newf( -			"reply policy '%s' not recognized on list %s", -			list.RepliesPolicy, list.ID, -		) -		return false, err +		log.Panicf(ctx, "unknown reply policy: %s", list.RepliesPolicy) +		return false, nil // unreachable code  	}  } @@ -452,6 +382,7 @@ func (s *Surface) timelineStatus(  	filters []*gtsmodel.Filter,  	mutes *usermute.CompiledUserMuteList,  ) (bool, error) { +  	// Ingest status into given timeline using provided function.  	if inserted, err := ingest(ctx, timelineID, status); err != nil {  		err = gtserror.Newf("error ingesting status %s: %w", status.ID, err) @@ -461,7 +392,7 @@ func (s *Surface) timelineStatus(  		return false, nil  	} -	// The status was inserted so stream it to the user. +	// Convert updated database model to frontend model.  	apiStatus, err := s.Converter.StatusToAPIStatus(ctx,  		status,  		account, @@ -473,6 +404,8 @@ func (s *Surface) timelineStatus(  		err = gtserror.Newf("error converting status %s to frontend representation: %w", status.ID, err)  		return true, err  	} + +	// The status was inserted so stream it to the user.  	s.Stream.Update(ctx, account, apiStatus, streamType)  	return true, nil @@ -492,7 +425,8 @@ func (s *Surface) timelineAndNotifyStatusForTagFollowers(  	}  	if status.BoostOf != nil { -		// Unwrap boost and work with the original status. +		// Unwrap boost and work +		// with the original status.  		status = status.BoostOf  	} @@ -523,6 +457,7 @@ func (s *Surface) timelineAndNotifyStatusForTagFollowers(  			)  		}  	} +  	return errs.Combine()  } @@ -667,17 +602,15 @@ func (s *Surface) timelineStatusUpdate(ctx context.Context, status *gtsmodel.Sta  		follows = append(follows, >smodel.Follow{  			AccountID:   status.AccountID,  			Account:     status.Account, -			Notify:      func() *bool { b := false; return &b }(), // Account shouldn't notify itself. -			ShowReblogs: func() *bool { b := true; return &b }(),  // Account should show own reblogs. +			Notify:      util.Ptr(false), // Account shouldn't notify itself. +			ShowReblogs: util.Ptr(true),  // Account should show own reblogs.  		})  	} -	// Push to streams for each local follower of this account. -	homeTimelinedAccountIDs, err := s.timelineStatusUpdateForFollowers(ctx, status, follows) -	if err != nil { -		return gtserror.Newf("error timelining status %s for followers: %w", status.ID, err) -	} +	// Push updated status to streams for each local follower of this account. +	homeTimelinedAccountIDs := s.timelineStatusUpdateForFollowers(ctx, status, follows) +	// Push updated status to streams for each local follower of tags in status, if applicable.  	if err := s.timelineStatusUpdateForTagFollowers(ctx, status, homeTimelinedAccountIDs); err != nil {  		return gtserror.Newf("error timelining status %s for tag followers: %w", status.ID, err)  	} @@ -695,12 +628,7 @@ func (s *Surface) timelineStatusUpdateForFollowers(  	ctx context.Context,  	status *gtsmodel.Status,  	follows []*gtsmodel.Follow, -) ([]string, error) { -	var ( -		errs                    gtserror.MultiError -		homeTimelinedAccountIDs = []string{} -	) - +) (homeTimelinedAccountIDs []string) {  	for _, follow := range follows {  		// Check to see if the status is timelineable for this follower,  		// taking account of its visibility, who it replies to, and, if @@ -718,7 +646,7 @@ func (s *Surface) timelineStatusUpdateForFollowers(  			ctx, follow.Account, status,  		)  		if err != nil { -			errs.Appendf("error checking status %s hometimelineability: %w", status.ID, err) +			log.Errorf(ctx, "error checking status home visibility for follow: %v", err)  			continue  		} @@ -727,31 +655,36 @@ func (s *Surface) timelineStatusUpdateForFollowers(  			continue  		} +		// Get relevant filters and mutes for this follow's account. +		// (note the origin account of the follow is receiver of status).  		filters, mutes, err := s.getFiltersAndMutes(ctx, follow.AccountID)  		if err != nil { -			errs.Append(err) +			log.Error(ctx, err)  			continue  		} -		// Add status to any relevant lists -		// for this follow, if applicable. -		exclusive := s.listTimelineStatusUpdateForFollow( -			ctx, +		// Add status to relevant lists for this follow, if applicable. +		_, exclusive, err := s.listTimelineStatusUpdateForFollow(ctx,  			status,  			follow, -			&errs,  			filters,  			mutes,  		) +		if err != nil { +			log.Errorf(ctx, "error list timelining status: %v", err) +			continue +		} +		// If this was timelined into +		// list with exclusive flag set, +		// don't add to home timeline.  		if exclusive {  			continue  		} -		// Add status to home timeline for owner -		// of this follow, if applicable. -		homeTimelined, err := s.timelineStreamStatusUpdate( -			ctx, +		// Add status to home timeline for owner of +		// this follow (origin account), if applicable. +		homeTimelined, err := s.timelineStreamStatusUpdate(ctx,  			follow.Account,  			status,  			stream.TimelineHome, @@ -759,15 +692,17 @@ func (s *Surface) timelineStatusUpdateForFollowers(  			mutes,  		)  		if err != nil { -			errs.Appendf("error home timelining status: %w", err) +			log.Errorf(ctx, "error home timelining status: %v", err)  			continue  		} +  		if homeTimelined { +			// If hometimelined, add to list of returned account IDs.  			homeTimelinedAccountIDs = append(homeTimelinedAccountIDs, follow.AccountID)  		}  	} -	return homeTimelinedAccountIDs, errs.Combine() +	return homeTimelinedAccountIDs  }  // listTimelineStatusUpdateForFollow pushes edits of the given status @@ -779,58 +714,59 @@ func (s *Surface) listTimelineStatusUpdateForFollow(  	ctx context.Context,  	status *gtsmodel.Status,  	follow *gtsmodel.Follow, -	errs *gtserror.MultiError,  	filters []*gtsmodel.Filter,  	mutes *usermute.CompiledUserMuteList, -) bool { -	// To put this status in appropriate list timelines, -	// we need to get each listEntry that pertains to -	// this follow. Then, we want to iterate through all -	// those list entries, and add the status to the list -	// that the entry belongs to if it meets criteria for -	// inclusion in the list. - -	listEntries, err := s.getListEntries(ctx, follow) -	if err != nil { -		errs.Append(err) -		return false -	} -	exclusive, err := s.isAnyListExclusive(ctx, listEntries) +) (bool, bool, error) { + +	// Get all lists that contain this given follow. +	lists, err := s.State.DB.GetListsContainingFollowID( + +		// We don't need list sub-models. +		gtscontext.SetBarebones(ctx), +		follow.ID, +	)  	if err != nil { -		errs.Append(err) -		return false +		return false, false, gtserror.Newf("error getting lists for follow: %w", err)  	} -	// Check eligibility for each list entry (if any). -	for _, listEntry := range listEntries { -		eligible, err := s.listEligible(ctx, listEntry, status) +	var exclusive, timelined bool +	for _, list := range lists { + +		// Check whether list is eligible for this status. +		eligible, err := s.listEligible(ctx, list, status)  		if err != nil { -			errs.Appendf("error checking list eligibility: %w", err) +			log.Errorf(ctx, "error checking list eligibility: %v", err)  			continue  		}  		if !eligible { -			// Don't add this.  			continue  		} +		// Update exclusive flag if list is so. +		exclusive = exclusive || *list.Exclusive +  		// At this point we are certain this status  		// should be included in the timeline of the  		// list that this list entry belongs to. -		if _, err := s.timelineStreamStatusUpdate( +		listTimelined, err := s.timelineStreamStatusUpdate(  			ctx,  			follow.Account,  			status, -			stream.TimelineList+":"+listEntry.ListID, // key streamType to this specific list +			stream.TimelineList+":"+list.ID, // key streamType to this specific list  			filters,  			mutes, -		); err != nil { -			errs.Appendf("error adding status to timeline for list %s: %w", listEntry.ListID, err) -			// implicit continue +		) +		if err != nil { +			log.Errorf(ctx, "error adding status to list timeline: %v", err) +			continue  		} + +		// Update flag based on if timelined. +		timelined = timelined || listTimelined  	} -	return exclusive +	return timelined, exclusive, nil  }  // timelineStatusUpdate streams the edited status to the user using the @@ -845,16 +781,31 @@ func (s *Surface) timelineStreamStatusUpdate(  	filters []*gtsmodel.Filter,  	mutes *usermute.CompiledUserMuteList,  ) (bool, error) { -	apiStatus, err := s.Converter.StatusToAPIStatus(ctx, status, account, statusfilter.FilterContextHome, filters, mutes) -	if errors.Is(err, statusfilter.ErrHideStatus) { + +	// Convert updated database model to frontend model. +	apiStatus, err := s.Converter.StatusToAPIStatus(ctx, +		status, +		account, +		statusfilter.FilterContextHome, +		filters, +		mutes, +	) + +	switch { +	case err == nil: +		// no issue. + +	case errors.Is(err, statusfilter.ErrHideStatus):  		// Don't put this status in the stream.  		return false, nil + +	default: +		return false, gtserror.Newf("error converting status: %w", err)  	} -	if err != nil { -		err = gtserror.Newf("error converting status %s to frontend representation: %w", status.ID, err) -		return false, err -	} + +	// The status was updated so stream it to the user.  	s.Stream.StatusUpdate(ctx, account, apiStatus, streamType) +  	return true, nil  } diff --git a/internal/processing/workers/util.go b/internal/processing/workers/util.go index 042f4827c..62ea6c95c 100644 --- a/internal/processing/workers/util.go +++ b/internal/processing/workers/util.go @@ -126,11 +126,6 @@ func (u *utils) wipeStatus(  			errs.Appendf("error deleting status poll: %w", err)  		} -		// Delete any poll votes pointing to this poll ID. -		if err := u.state.DB.DeletePollVotes(ctx, pollID); err != nil { -			errs.Appendf("error deleting status poll votes: %w", err) -		} -  		// Cancel any scheduled expiry task for poll.  		_ = u.state.Workers.Scheduler.Cancel(pollID)  	}  | 
