From 84279f6a6a0201c90a6747fe8b82c38d5b4e49e2 Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Mon, 16 Sep 2024 16:46:09 +0000 Subject: [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 --- internal/api/client/exports/exports_test.go | 4 +-- internal/api/client/lists/listaccounts.go | 36 +++++++---------------- internal/api/client/lists/listaccounts_test.go | 13 ++++---- internal/api/client/lists/listaccountsadd_test.go | 7 +++-- 4 files changed, 23 insertions(+), 37 deletions(-) (limited to 'internal/api') diff --git a/internal/api/client/exports/exports_test.go b/internal/api/client/exports/exports_test.go index 1943f2582..303361b86 100644 --- a/internal/api/client/exports/exports_test.go +++ b/internal/api/client/exports/exports_test.go @@ -188,8 +188,8 @@ admin@localhost:8080 token: suite.testTokens["local_account_1"], user: suite.testUsers["local_account_1"], account: suite.testAccounts["local_account_1"], - expect: `Cool Ass Posters From This Instance,admin@localhost:8080 -Cool Ass Posters From This Instance,1happyturtle@localhost:8080 + expect: `Cool Ass Posters From This Instance,1happyturtle@localhost:8080 +Cool Ass Posters From This Instance,admin@localhost:8080 `, }, // Export Mutes. diff --git a/internal/api/client/lists/listaccounts.go b/internal/api/client/lists/listaccounts.go index e1d340ebb..d609251f7 100644 --- a/internal/api/client/lists/listaccounts.go +++ b/internal/api/client/lists/listaccounts.go @@ -25,6 +25,7 @@ import ( apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/oauth" + "github.com/superseriousbusiness/gotosocial/internal/paging" ) // ListAccountsGETHandler swagger:operation GET /api/v1/lists/{id}/accounts listAccounts @@ -129,42 +130,27 @@ func (m *Module) ListAccountsGETHandler(c *gin.Context) { targetListID := c.Param(IDKey) if targetListID == "" { - err := errors.New("no list id specified") - apiutil.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGetV1) - return - } - - limit, errWithCode := apiutil.ParseLimit(c.Query(apiutil.LimitKey), 40, 80, 0) - if errWithCode != nil { + const text = "no list id specified" + errWithCode := gtserror.NewErrorBadRequest(errors.New(text), text) apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1) return } - var ( - ctx = c.Request.Context() + page, errWithCode := paging.ParseIDPage(c, + 1, // min limit + 80, // max limit + 0, // default = paging disabled ) - - if limit == 0 { - // Return all accounts in the list without pagination. - accounts, errWithCode := m.processor.List().GetAllListAccounts(ctx, authed.Account, targetListID) - if errWithCode != nil { - apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1) - return - } - - c.JSON(http.StatusOK, accounts) + if errWithCode != nil { + apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1) return } - // Return subset of accounts in the list with pagination. resp, errWithCode := m.processor.List().GetListAccounts( - ctx, + c.Request.Context(), authed.Account, targetListID, - c.Query(MaxIDKey), - c.Query(SinceIDKey), - c.Query(MinIDKey), - limit, + page, ) if errWithCode != nil { apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1) diff --git a/internal/api/client/lists/listaccounts_test.go b/internal/api/client/lists/listaccounts_test.go index bbd187f7d..e0a16e29f 100644 --- a/internal/api/client/lists/listaccounts_test.go +++ b/internal/api/client/lists/listaccounts_test.go @@ -19,7 +19,7 @@ package lists_test import ( "encoding/json" - "io/ioutil" + "io" "net/http" "net/http/httptest" "strconv" @@ -97,7 +97,7 @@ func (suite *ListAccountsTestSuite) getListAccounts( result := recorder.Result() defer result.Body.Close() - b, err := ioutil.ReadAll(result.Body) + b, err := io.ReadAll(result.Body) if err != nil { return nil, "", err } @@ -151,8 +151,7 @@ func (suite *ListAccountsTestSuite) TestGetListAccountsPaginatedDefaultLimit() { suite.Len(accounts, 2) suite.Equal( - `; rel="next", `+ - `; rel="prev"`, + "; rel=\"next\", ; rel=\"prev\"", link, ) } @@ -184,8 +183,7 @@ func (suite *ListAccountsTestSuite) TestGetListAccountsPaginatedNextPage() { suite.Len(accounts, 1) suite.Equal( - `; rel="next", `+ - `; rel="prev"`, + "; rel=\"next\", ; rel=\"prev\"", link, ) @@ -206,8 +204,7 @@ func (suite *ListAccountsTestSuite) TestGetListAccountsPaginatedNextPage() { suite.Len(accounts, 1) suite.Equal( - `; rel="next", `+ - `; rel="prev"`, + "; rel=\"next\", ; rel=\"prev\"", link, ) } diff --git a/internal/api/client/lists/listaccountsadd_test.go b/internal/api/client/lists/listaccountsadd_test.go index 7e44eeed3..e71cf0992 100644 --- a/internal/api/client/lists/listaccountsadd_test.go +++ b/internal/api/client/lists/listaccountsadd_test.go @@ -98,14 +98,17 @@ func (suite *ListAccountsAddTestSuite) TestPostListAccountNotFollowed() { resp, err := suite.postListAccounts(http.StatusNotFound, listID, accountIDs) suite.NoError(err) - suite.Equal(`{"error":"Not Found: you do not follow account 01F8MH5ZK5VRH73AKHQM6Y9VNX"}`, string(resp)) + suite.Equal(`{"error":"Not Found: account 01F8MH5ZK5VRH73AKHQM6Y9VNX not currently followed"}`, string(resp)) } func (suite *ListAccountsAddTestSuite) TestPostListAccountOK() { + entry := suite.testListEntries["local_account_1_list_1_entry_1"] + // Remove turtle from the list. if err := suite.db.DeleteListEntry( context.Background(), - suite.testListEntries["local_account_1_list_1_entry_1"].ID, + entry.ListID, + entry.FollowID, ); err != nil { suite.FailNow(err.Error()) } -- cgit v1.2.3