From ff406be68f0fe6fc0b2dae9a091ce164ac039b3f Mon Sep 17 00:00:00 2001 From: Tobi Smethurst <31960611+tsmethurst@users.noreply.github.com> Date: Sun, 15 Aug 2021 18:43:08 +0200 Subject: [PATCH] Timeline loop fix (#140) * uwu we made a fucky wucky * uwu we made a fucky wucky * work on timeline fixes a little * fiddle with tests some more * bleep bloop more tests * more tests * update drone yml * update some sturf * make the timeline code a bit lazier * go fmt * fix drone.yml --- .drone.yml | 29 +- internal/db/pg/timeline.go | 19 +- internal/processing/processor.go | 2 +- internal/processing/timeline.go | 102 ------- internal/timeline/get.go | 90 +++--- internal/timeline/get_test.go | 438 +++++++++++++++++++++++++++++ internal/timeline/index.go | 95 +++++-- internal/timeline/index_test.go | 193 +++++++++++++ internal/timeline/manager.go | 10 +- internal/timeline/manager_test.go | 142 ++++++++++ internal/timeline/prepare.go | 39 ++- internal/timeline/timeline.go | 13 +- internal/timeline/timeline_test.go | 43 +++ 13 files changed, 1035 insertions(+), 180 deletions(-) create mode 100644 internal/timeline/get_test.go create mode 100644 internal/timeline/index_test.go create mode 100644 internal/timeline/manager_test.go create mode 100644 internal/timeline/timeline_test.go diff --git a/.drone.yml b/.drone.yml index 42ec83ce4..f6c257216 100644 --- a/.drone.yml +++ b/.drone.yml @@ -14,6 +14,11 @@ steps: # See: https://golangci-lint.run/ - name: lint image: golangci/golangci-lint:v1.41.1 + volumes: + - name: go-build-cache + path: /root/.cache/go-build + - name: golangci-lint-cache + path: /root/.cache/golangci-lint commands: - golangci-lint run --timeout 5m0s --tests=false --verbose when: @@ -23,6 +28,9 @@ steps: - name: test image: golang:1.16.4 + volumes: + - name: go-build-cache + path: /root/.cache/go-build environment: GTS_DB_ADDRESS: postgres commands: @@ -49,15 +57,30 @@ steps: exclude: - pull_request -services: -# We need this postgres service running for the test step. +# We need a postgres service running for the test step. # See: https://docs.drone.io/pipeline/docker/syntax/services/ +services: - name: postgres image: postgres environment: POSTGRES_PASSWORD: postgres + when: + event: + include: + - pull_request + +# We can speed up builds significantly by caching build artifacts between runs. +# See: https://docs.drone.io/pipeline/docker/syntax/volumes/host/ +volumes: +- name: go-build-cache + host: + path: /drone/gotosocial/go-build +- name: golangci-lint-cache + host: + path: /drone/gotosocial/golangci-lint + --- kind: signature -hmac: 888b0a9964be9bddad325a8eab0f54350f3cd36c333564ad4333811b4841b640 +hmac: 9134975e238ab9f92a7f75ccc11279e8d5edddb87f10165ed3c7d23fdd9c8a11 ... diff --git a/internal/db/pg/timeline.go b/internal/db/pg/timeline.go index 95acd4f38..585ca3067 100644 --- a/internal/db/pg/timeline.go +++ b/internal/db/pg/timeline.go @@ -28,31 +28,46 @@ import ( func (ps *postgresService) GetHomeTimelineForAccount(accountID string, maxID string, sinceID string, minID string, limit int, local bool) ([]*gtsmodel.Status, error) { statuses := []*gtsmodel.Status{} - q := ps.conn.Model(&statuses) q = q.ColumnExpr("status.*"). + // Find out who accountID follows. Join("LEFT JOIN follows AS f ON f.target_account_id = status.account_id"). - Where("f.account_id = ?", accountID). + // Use a WhereGroup here to specify that we want EITHER statuses posted by accounts that accountID follows, + // OR statuses posted by accountID itself (since a user should be able to see their own statuses). + // + // This is equivalent to something like WHERE ... AND (... OR ...) + // See: https://pg.uptrace.dev/queries/#select + WhereGroup(func(q *pg.Query) (*pg.Query, error) { + q = q.WhereOr("f.account_id = ?", accountID). + WhereOr("status.account_id = ?", accountID) + return q, nil + }). + // Sort by highest ID (newest) to lowest ID (oldest) Order("status.id DESC") if maxID != "" { + // return only statuses LOWER (ie., older) than maxID q = q.Where("status.id < ?", maxID) } if sinceID != "" { + // return only statuses HIGHER (ie., newer) than sinceID q = q.Where("status.id > ?", sinceID) } if minID != "" { + // return only statuses HIGHER (ie., newer) than minID q = q.Where("status.id > ?", minID) } if local { + // return only statuses posted by local account havers q = q.Where("status.local = ?", local) } if limit > 0 { + // limit amount of statuses returned q = q.Limit(limit) } diff --git a/internal/processing/processor.go b/internal/processing/processor.go index 16f9ac2a3..48ed2a35f 100644 --- a/internal/processing/processor.go +++ b/internal/processing/processor.go @@ -304,7 +304,7 @@ func (p *processor) Start() error { } } }() - return p.initTimelines() + return nil } // Stop stops the processor cleanly, finishing handling any remaining messages before closing down. diff --git a/internal/processing/timeline.go b/internal/processing/timeline.go index b5cbf433a..18d0a6ac7 100644 --- a/internal/processing/timeline.go +++ b/internal/processing/timeline.go @@ -21,9 +21,7 @@ package processing import ( "fmt" "net/url" - "sync" - "github.com/sirupsen/logrus" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtserror" @@ -186,103 +184,3 @@ func (p *processor) filterFavedStatuses(authed *oauth.Auth, statuses []*gtsmodel return apiStatuses, nil } - -func (p *processor) initTimelines() error { - // get all local accounts (ie., domain = nil) that aren't suspended (suspended_at = nil) - localAccounts := []*gtsmodel.Account{} - where := []db.Where{ - { - Key: "domain", Value: nil, - }, - { - Key: "suspended_at", Value: nil, - }, - } - if err := p.db.GetWhere(where, &localAccounts); err != nil { - if _, ok := err.(db.ErrNoEntries); ok { - return nil - } - return fmt.Errorf("initTimelines: db error initializing timelines: %s", err) - } - - // we want to wait until all timelines are populated so created a waitgroup here - wg := &sync.WaitGroup{} - wg.Add(len(localAccounts)) - - for _, localAccount := range localAccounts { - // to save time we can populate the timelines asynchronously - // this will go heavy on the database, but since we're not actually serving yet it doesn't really matter - go p.initTimelineFor(localAccount, wg) - } - - // wait for all timelines to be populated before we exit - wg.Wait() - return nil -} - -func (p *processor) initTimelineFor(account *gtsmodel.Account, wg *sync.WaitGroup) { - defer wg.Done() - - l := p.log.WithFields(logrus.Fields{ - "func": "initTimelineFor", - "accountID": account.ID, - }) - - desiredIndexLength := p.timelineManager.GetDesiredIndexLength() - - statuses, err := p.db.GetHomeTimelineForAccount(account.ID, "", "", "", desiredIndexLength, false) - if err != nil { - if _, ok := err.(db.ErrNoEntries); !ok { - l.Error(fmt.Errorf("initTimelineFor: error getting statuses: %s", err)) - } - return - } - p.indexAndIngest(statuses, account, desiredIndexLength) - - lengthNow := p.timelineManager.GetIndexedLength(account.ID) - if lengthNow < desiredIndexLength { - // try and get more posts from the last ID onwards - rearmostStatusID, err := p.timelineManager.GetOldestIndexedID(account.ID) - if err != nil { - l.Error(fmt.Errorf("initTimelineFor: error getting id of rearmost status: %s", err)) - return - } - - if rearmostStatusID != "" { - moreStatuses, err := p.db.GetHomeTimelineForAccount(account.ID, rearmostStatusID, "", "", desiredIndexLength/2, false) - if err != nil { - l.Error(fmt.Errorf("initTimelineFor: error getting more statuses: %s", err)) - return - } - p.indexAndIngest(moreStatuses, account, desiredIndexLength) - } - } - - l.Debugf("prepared timeline of length %d for account %s", lengthNow, account.ID) -} - -func (p *processor) indexAndIngest(statuses []*gtsmodel.Status, timelineAccount *gtsmodel.Account, desiredIndexLength int) { - l := p.log.WithFields(logrus.Fields{ - "func": "indexAndIngest", - "accountID": timelineAccount.ID, - }) - - for _, s := range statuses { - timelineable, err := p.filter.StatusHometimelineable(s, timelineAccount) - if err != nil { - l.Error(fmt.Errorf("initTimelineFor: error checking home timelineability of status %s: %s", s.ID, err)) - continue - } - if timelineable { - if _, err := p.timelineManager.Ingest(s, timelineAccount.ID); err != nil { - l.Error(fmt.Errorf("initTimelineFor: error ingesting status %s: %s", s.ID, err)) - continue - } - - // check if we have enough posts now and return if we do - if p.timelineManager.GetIndexedLength(timelineAccount.ID) >= desiredIndexLength { - return - } - } - } -} diff --git a/internal/timeline/get.go b/internal/timeline/get.go index 9d6c6efbf..d800da4e3 100644 --- a/internal/timeline/get.go +++ b/internal/timeline/get.go @@ -29,7 +29,7 @@ import ( const retries = 5 -func (t *timeline) Get(amount int, maxID string, sinceID string, minID string) ([]*apimodel.Status, error) { +func (t *timeline) Get(amount int, maxID string, sinceID string, minID string, prepareNext bool) ([]*apimodel.Status, error) { l := t.log.WithFields(logrus.Fields{ "func": "Get", "accountID": t.accountID, @@ -44,35 +44,44 @@ func (t *timeline) Get(amount int, maxID string, sinceID string, minID string) ( var err error // no params are defined to just fetch from the top + // this is equivalent to a user asking for the top x posts from their timeline if maxID == "" && sinceID == "" && minID == "" { statuses, err = t.GetXFromTop(amount) // aysnchronously prepare the next predicted query so it's ready when the user asks for it if len(statuses) != 0 { nextMaxID := statuses[len(statuses)-1].ID - go func() { - if err := t.prepareNextQuery(amount, nextMaxID, "", ""); err != nil { - l.Errorf("error preparing next query: %s", err) - } - }() + if prepareNext { + // already cache the next query to speed up scrolling + go func() { + if err := t.prepareNextQuery(amount, nextMaxID, "", ""); err != nil { + l.Errorf("error preparing next query: %s", err) + } + }() + } } } // maxID is defined but sinceID isn't so take from behind + // this is equivalent to a user asking for the next x posts from their timeline, starting from maxID if maxID != "" && sinceID == "" { attempts := 0 statuses, err = t.GetXBehindID(amount, maxID, &attempts) // aysnchronously prepare the next predicted query so it's ready when the user asks for it if len(statuses) != 0 { nextMaxID := statuses[len(statuses)-1].ID - go func() { - if err := t.prepareNextQuery(amount, nextMaxID, "", ""); err != nil { - l.Errorf("error preparing next query: %s", err) - } - }() + if prepareNext { + // already cache the next query to speed up scrolling + go func() { + if err := t.prepareNextQuery(amount, nextMaxID, "", ""); err != nil { + l.Errorf("error preparing next query: %s", err) + } + }() + } } } // maxID is defined and sinceID || minID are as well, so take a slice between them + // this is equivalent to a user asking for posts older than x but newer than y if maxID != "" && sinceID != "" { statuses, err = t.GetXBetweenID(amount, maxID, minID) } @@ -81,13 +90,12 @@ func (t *timeline) Get(amount int, maxID string, sinceID string, minID string) ( } // maxID isn't defined, but sinceID || minID are, so take x before + // this is equivalent to a user asking for posts newer than x (eg., refreshing the top of their timeline) if maxID == "" && sinceID != "" { - attempts := 0 - statuses, err = t.GetXBeforeID(amount, sinceID, true, &attempts) + statuses, err = t.GetXBeforeID(amount, sinceID, true) } if maxID == "" && minID != "" { - attempts := 0 - statuses, err = t.GetXBeforeID(amount, minID, true, &attempts) + statuses, err = t.GetXBeforeID(amount, minID, true) } return statuses, err @@ -126,6 +134,13 @@ func (t *timeline) GetXFromTop(amount int) ([]*apimodel.Status, error) { } func (t *timeline) GetXBehindID(amount int, behindID string, attempts *int) ([]*apimodel.Status, error) { + l := t.log.WithFields(logrus.Fields{ + "func": "GetXBehindID", + "amount": amount, + "behindID": behindID, + "attempts": *attempts, + }) + newAttempts := *attempts newAttempts = newAttempts + 1 attempts = &newAttempts @@ -149,17 +164,16 @@ findMarkLoop: return nil, errors.New("GetXBehindID: could not parse e as a preparedPostsEntry") } - if entry.statusID == behindID { + if entry.statusID <= behindID { + l.Trace("found behindID mark") behindIDMark = e break findMarkLoop } } // we didn't find it, so we need to make sure it's indexed and prepared and then try again + // this can happen when a user asks for really old posts if behindIDMark == nil { - if err := t.IndexBehind(behindID, amount); err != nil { - return nil, fmt.Errorf("GetXBehindID: error indexing behind and including ID %s", behindID) - } if err := t.PrepareBehind(behindID, amount); err != nil { return nil, fmt.Errorf("GetXBehindID: error preparing behind and including ID %s", behindID) } @@ -167,12 +181,19 @@ findMarkLoop: if err != nil { return nil, err } - if oldestID == "" || oldestID == behindID || *attempts > retries { - // There is no oldest prepared post, or the oldest prepared post is still the post we're looking for entries after, - // or we've tried this loop too many times. - // This means we should just return the empty statuses slice since we don't have any more posts to offer. + if oldestID == "" { + l.Tracef("oldestID is empty so we can't return behindID %s", behindID) return statuses, nil } + if oldestID == behindID { + l.Tracef("given behindID %s is the same as oldestID %s so there's nothing to return behind it", behindID, oldestID) + return statuses, nil + } + if *attempts > retries { + l.Tracef("exceeded retries looking for behindID %s", behindID) + return statuses, nil + } + l.Trace("trying GetXBehindID again") return t.GetXBehindID(amount, behindID, attempts) } @@ -203,11 +224,7 @@ serveloop: return statuses, nil } -func (t *timeline) GetXBeforeID(amount int, beforeID string, startFromTop bool, attempts *int) ([]*apimodel.Status, error) { - newAttempts := *attempts - newAttempts = newAttempts + 1 - attempts = &newAttempts - +func (t *timeline) GetXBeforeID(amount int, beforeID string, startFromTop bool) ([]*apimodel.Status, error) { // make a slice of statuses with the length we need to return statuses := make([]*apimodel.Status, 0, amount) @@ -215,7 +232,7 @@ func (t *timeline) GetXBeforeID(amount int, beforeID string, startFromTop bool, t.preparedPosts.data = &list.List{} } - // iterate through the modified list until we hit the mark we're looking for + // iterate through the modified list until we hit the mark we're looking for, or as close as possible to it var beforeIDMark *list.Element findMarkLoop: for e := t.preparedPosts.data.Front(); e != nil; e = e.Next() { @@ -224,24 +241,15 @@ findMarkLoop: return nil, errors.New("GetXBeforeID: could not parse e as a preparedPostsEntry") } - if entry.statusID == beforeID { + if entry.statusID >= beforeID { beforeIDMark = e + } else { break findMarkLoop } } - // we didn't find it, so we need to make sure it's indexed and prepared and then try again if beforeIDMark == nil { - if err := t.IndexBefore(beforeID, true, amount); err != nil { - return nil, fmt.Errorf("GetXBeforeID: error indexing before and including ID %s", beforeID) - } - if err := t.PrepareBefore(beforeID, true, amount); err != nil { - return nil, fmt.Errorf("GetXBeforeID: error preparing before and including ID %s", beforeID) - } - if *attempts > retries { - return statuses, nil - } - return t.GetXBeforeID(amount, beforeID, startFromTop, attempts) + return statuses, nil } var served int diff --git a/internal/timeline/get_test.go b/internal/timeline/get_test.go new file mode 100644 index 000000000..0866f3bdd --- /dev/null +++ b/internal/timeline/get_test.go @@ -0,0 +1,438 @@ +/* + GoToSocial + Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see . +*/ + +package timeline_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/timeline" + "github.com/superseriousbusiness/gotosocial/testrig" +) + +type GetTestSuite struct { + TimelineStandardTestSuite +} + +func (suite *GetTestSuite) SetupSuite() { + suite.testAccounts = testrig.NewTestAccounts() + suite.testStatuses = testrig.NewTestStatuses() +} + +func (suite *GetTestSuite) SetupTest() { + suite.config = testrig.NewTestConfig() + suite.db = testrig.NewTestDB() + suite.log = testrig.NewTestLog() + suite.tc = testrig.NewTestTypeConverter(suite.db) + + testrig.StandardDBSetup(suite.db, nil) + + // let's take local_account_1 as the timeline owner + tl, err := timeline.NewTimeline(suite.testAccounts["local_account_1"].ID, suite.db, suite.tc, suite.log) + if err != nil { + suite.FailNow(err.Error()) + } + + // prepare the timeline by just shoving all test statuses in it -- let's not be fussy about who sees what + for _, s := range suite.testStatuses { + _, err := tl.IndexAndPrepareOne(s.CreatedAt, s.ID, s.BoostOfID, s.AccountID, s.BoostOfAccountID) + if err != nil { + suite.FailNow(err.Error()) + } + } + + suite.timeline = tl +} + +func (suite *GetTestSuite) TearDownTest() { + testrig.StandardDBTeardown(suite.db) +} + +func (suite *GetTestSuite) TestGetDefault() { + // get 10 20 the top and don't prepare the next query + statuses, err := suite.timeline.Get(20, "", "", "", false) + if err != nil { + suite.FailNow(err.Error()) + } + + // we only have 12 statuses in the test suite + suite.Len(statuses, 12) + + // statuses should be sorted highest to lowest ID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + } +} + +func (suite *GetTestSuite) TestGetDefaultPrepareNext() { + // get 10 from the top and prepare the next query + statuses, err := suite.timeline.Get(10, "", "", "", true) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Len(statuses, 10) + + // statuses should be sorted highest to lowest ID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + } + + // sleep a second so the next query can run + time.Sleep(1 * time.Second) +} + +func (suite *GetTestSuite) TestGetMaxID() { + // ask for 10 with a max ID somewhere in the middle of the stack + statuses, err := suite.timeline.Get(10, "01F8MHBQCBTDKN6X5VHGMMN4MA", "", "", false) + if err != nil { + suite.FailNow(err.Error()) + } + + // we should only get 6 statuses back, since we asked for a max ID that excludes some of our entries + suite.Len(statuses, 6) + + // statuses should be sorted highest to lowest ID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + } +} + +func (suite *GetTestSuite) TestGetMaxIDPrepareNext() { + // ask for 10 with a max ID somewhere in the middle of the stack + statuses, err := suite.timeline.Get(10, "01F8MHBQCBTDKN6X5VHGMMN4MA", "", "", true) + if err != nil { + suite.FailNow(err.Error()) + } + + // we should only get 6 statuses back, since we asked for a max ID that excludes some of our entries + suite.Len(statuses, 6) + + // statuses should be sorted highest to lowest ID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + } + + // sleep a second so the next query can run + time.Sleep(1 * time.Second) +} + +func (suite *GetTestSuite) TestGetMinID() { + // ask for 10 with a min ID somewhere in the middle of the stack + statuses, err := suite.timeline.Get(10, "", "01F8MHBQCBTDKN6X5VHGMMN4MA", "", false) + if err != nil { + suite.FailNow(err.Error()) + } + + // we should only get 5 statuses back, since we asked for a min ID that excludes some of our entries + suite.Len(statuses, 5) + + // statuses should be sorted highest to lowest ID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + } +} + +func (suite *GetTestSuite) TestGetSinceID() { + // ask for 10 with a since ID somewhere in the middle of the stack + statuses, err := suite.timeline.Get(10, "", "", "01F8MHBQCBTDKN6X5VHGMMN4MA", false) + if err != nil { + suite.FailNow(err.Error()) + } + + // we should only get 5 statuses back, since we asked for a since ID that excludes some of our entries + suite.Len(statuses, 5) + + // statuses should be sorted highest to lowest ID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + } +} + +func (suite *GetTestSuite) TestGetSinceIDPrepareNext() { + // ask for 10 with a since ID somewhere in the middle of the stack + statuses, err := suite.timeline.Get(10, "", "", "01F8MHBQCBTDKN6X5VHGMMN4MA", true) + if err != nil { + suite.FailNow(err.Error()) + } + + // we should only get 5 statuses back, since we asked for a since ID that excludes some of our entries + suite.Len(statuses, 5) + + // statuses should be sorted highest to lowest ID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + } + + // sleep a second so the next query can run + time.Sleep(1 * time.Second) +} + +func (suite *GetTestSuite) TestGetBetweenID() { + // ask for 10 between these two IDs + statuses, err := suite.timeline.Get(10, "01F8MHCP5P2NWYQ416SBA0XSEV", "", "01F8MHBQCBTDKN6X5VHGMMN4MA", false) + if err != nil { + suite.FailNow(err.Error()) + } + + // we should only get 2 statuses back, since there are only two statuses between the given IDs + suite.Len(statuses, 2) + + // statuses should be sorted highest to lowest ID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + } +} + +func (suite *GetTestSuite) TestGetBetweenIDPrepareNext() { + // ask for 10 between these two IDs + statuses, err := suite.timeline.Get(10, "01F8MHCP5P2NWYQ416SBA0XSEV", "", "01F8MHBQCBTDKN6X5VHGMMN4MA", true) + if err != nil { + suite.FailNow(err.Error()) + } + + // we should only get 2 statuses back, since there are only two statuses between the given IDs + suite.Len(statuses, 2) + + // statuses should be sorted highest to lowest ID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + } + + // sleep a second so the next query can run + time.Sleep(1 * time.Second) +} + +func (suite *GetTestSuite) TestGetXFromTop() { + // get 5 from the top + statuses, err := suite.timeline.GetXFromTop(5) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Len(statuses, 5) + + // statuses should be sorted highest to lowest ID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + } +} + +func (suite *GetTestSuite) TestGetXBehindID() { + // get 3 behind the 'middle' id + var attempts *int + a := 0 + attempts = &a + statuses, err := suite.timeline.GetXBehindID(3, "01F8MHBQCBTDKN6X5VHGMMN4MA", attempts) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Len(statuses, 3) + + // statuses should be sorted highest to lowest ID + // all status IDs should be less than the behindID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + suite.Less(s.ID, "01F8MHBQCBTDKN6X5VHGMMN4MA") + } +} + +func (suite *GetTestSuite) TestGetXBehindID0() { + // try to get behind 0, the lowest possible ID + var attempts *int + a := 0 + attempts = &a + statuses, err := suite.timeline.GetXBehindID(3, "0", attempts) + if err != nil { + suite.FailNow(err.Error()) + } + + // there's nothing beyond it so len should be 0 + suite.Len(statuses, 0) +} + +func (suite *GetTestSuite) TestGetXBehindNonexistentReasonableID() { + // try to get behind an id that doesn't exist, but is close to one that does so we should still get statuses back + var attempts *int + a := 0 + attempts = &a + statuses, err := suite.timeline.GetXBehindID(3, "01F8MHBQCBTDKN6X5VHGMMN4MB", attempts) // change the last A to a B + if err != nil { + suite.FailNow(err.Error()) + } + suite.Len(statuses, 3) + + // statuses should be sorted highest to lowest ID + // all status IDs should be less than the behindID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + suite.Less(s.ID, "01F8MHBCN8120SYH7D5S050MGK") + } +} + +func (suite *GetTestSuite) TestGetXBehindVeryHighID() { + // try to get behind an id that doesn't exist, and is higher than any other ID we could possibly have + var attempts *int + a := 0 + attempts = &a + statuses, err := suite.timeline.GetXBehindID(7, "9998MHBQCBTDKN6X5VHGMMN4MA", attempts) + if err != nil { + suite.FailNow(err.Error()) + } + + // we should get all 7 statuses we asked for because they all have lower IDs than the very high ID given in the query + suite.Len(statuses, 7) + + // statuses should be sorted highest to lowest ID + // all status IDs should be less than the behindID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + suite.Less(s.ID, "9998MHBQCBTDKN6X5VHGMMN4MA") + } +} + +func (suite *GetTestSuite) TestGetXBeforeID() { + // get 3 before the 'middle' id + statuses, err := suite.timeline.GetXBeforeID(3, "01F8MHBQCBTDKN6X5VHGMMN4MA", true) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Len(statuses, 3) + + // statuses should be sorted highest to lowest ID + // all status IDs should be greater than the beforeID + var highest string + for i, s := range statuses { + if i == 0 { + highest = s.ID + } else { + suite.Less(s.ID, highest) + highest = s.ID + } + suite.Greater(s.ID, "01F8MHBQCBTDKN6X5VHGMMN4MA") + } +} + +func (suite *GetTestSuite) TestGetXBeforeIDNoStartFromTop() { + // get 3 before the 'middle' id + statuses, err := suite.timeline.GetXBeforeID(3, "01F8MHBQCBTDKN6X5VHGMMN4MA", false) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Len(statuses, 3) + + // statuses should be sorted lowest to highest ID + // all status IDs should be greater than the beforeID + var lowest string + for i, s := range statuses { + if i == 0 { + lowest = s.ID + } else { + suite.Greater(s.ID, lowest) + lowest = s.ID + } + suite.Greater(s.ID, "01F8MHBQCBTDKN6X5VHGMMN4MA") + } +} + +func TestGetTestSuite(t *testing.T) { + suite.Run(t, new(GetTestSuite)) +} diff --git a/internal/timeline/index.go b/internal/timeline/index.go index c8894b284..1e1a9d7bb 100644 --- a/internal/timeline/index.go +++ b/internal/timeline/index.go @@ -19,30 +19,38 @@ package timeline import ( + "container/list" "errors" "fmt" "time" + "github.com/sirupsen/logrus" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) func (t *timeline) IndexBefore(statusID string, include bool, amount int) error { + // lazily initialize index if it hasn't been done already + if t.postIndex.data == nil { + t.postIndex.data = &list.List{} + t.postIndex.data.Init() + } + filtered := []*gtsmodel.Status{} offsetStatus := statusID if include { + // if we have the status with given statusID in the database, include it in the results set as well s := >smodel.Status{} - if err := t.db.GetByID(statusID, s); err != nil { - return fmt.Errorf("IndexBefore: error getting initial status with id %s: %s", statusID, err) + if err := t.db.GetByID(statusID, s); err == nil { + filtered = append(filtered, s) } - filtered = append(filtered, s) } i := 0 grabloop: for ; len(filtered) < amount && i < 5; i = i + 1 { // try the grabloop 5 times only - statuses, err := t.db.GetHomeTimelineForAccount(t.accountID, "", offsetStatus, "", amount, false) + statuses, err := t.db.GetHomeTimelineForAccount(t.accountID, "", "", offsetStatus, amount, false) if err != nil { if _, ok := err.(db.ErrNoEntries); ok { break grabloop // we just don't have enough statuses left in the db so index what we've got and then bail @@ -71,24 +79,70 @@ grabloop: return nil } -func (t *timeline) IndexBehind(statusID string, amount int) error { +func (t *timeline) IndexBehind(statusID string, include bool, amount int) error { + l := t.log.WithFields(logrus.Fields{ + "func": "IndexBehind", + "include": include, + "amount": amount, + }) + + // lazily initialize index if it hasn't been done already + if t.postIndex.data == nil { + t.postIndex.data = &list.List{} + t.postIndex.data.Init() + } + + // If we're already indexedBehind given statusID by the required amount, we can return nil. + // First find position of statusID (or as near as possible). + var position int +positionLoop: + for e := t.postIndex.data.Front(); e != nil; e = e.Next() { + entry, ok := e.Value.(*postIndexEntry) + if !ok { + return errors.New("IndexBehind: could not parse e as a postIndexEntry") + } + + if entry.statusID <= statusID { + // we've found it + break positionLoop + } + position++ + } + // now check if the length of indexed posts exceeds the amount of posts required (position of statusID, plus amount of posts requested after that) + if t.postIndex.data.Len() > position+amount { + // we have enough indexed behind already to satisfy amount, so don't need to make db calls + l.Trace("returning nil since we already have enough posts indexed") + return nil + } + filtered := []*gtsmodel.Status{} offsetStatus := statusID + if include { + // if we have the status with given statusID in the database, include it in the results set as well + s := >smodel.Status{} + if err := t.db.GetByID(statusID, s); err == nil { + filtered = append(filtered, s) + } + } + i := 0 grabloop: for ; len(filtered) < amount && i < 5; i = i + 1 { // try the grabloop 5 times only + l.Tracef("entering grabloop; i is %d; len(filtered) is %d", i, len(filtered)) statuses, err := t.db.GetHomeTimelineForAccount(t.accountID, offsetStatus, "", "", amount, false) if err != nil { if _, ok := err.(db.ErrNoEntries); ok { break grabloop // we just don't have enough statuses left in the db so index what we've got and then bail } - return fmt.Errorf("IndexBehindAndIncluding: error getting statuses from db: %s", err) + return fmt.Errorf("IndexBehind: error getting statuses from db: %s", err) } + l.Tracef("got %d statuses", len(statuses)) for _, s := range statuses { timelineable, err := t.filter.StatusHometimelineable(s, t.account) if err != nil { + l.Tracef("status was not hometimelineable: %s", err) continue } if timelineable { @@ -97,6 +151,7 @@ grabloop: offsetStatus = s.ID } } + l.Trace("left grabloop") for _, s := range filtered { if _, err := t.IndexOne(s.CreatedAt, s.ID, s.BoostOfID, s.AccountID, s.BoostOfAccountID); err != nil { @@ -104,10 +159,7 @@ grabloop: } } - return nil -} - -func (t *timeline) IndexOneByID(statusID string) error { + l.Trace("exiting function") return nil } @@ -152,21 +204,30 @@ func (t *timeline) IndexAndPrepareOne(statusCreatedAt time.Time, statusID string func (t *timeline) OldestIndexedPostID() (string, error) { var id string - if t.postIndex == nil || t.postIndex.data == nil { + if t.postIndex == nil || t.postIndex.data == nil || t.postIndex.data.Back() == nil { // return an empty string if postindex hasn't been initialized yet return id, nil } e := t.postIndex.data.Back() - - if e == nil { - // return an empty string if there's no back entry (ie., the index list hasn't been initialized yet) - return id, nil - } - entry, ok := e.Value.(*postIndexEntry) if !ok { return id, errors.New("OldestIndexedPostID: could not parse e as a postIndexEntry") } return entry.statusID, nil } + +func (t *timeline) NewestIndexedPostID() (string, error) { + var id string + if t.postIndex == nil || t.postIndex.data == nil || t.postIndex.data.Front() == nil { + // return an empty string if postindex hasn't been initialized yet + return id, nil + } + + e := t.postIndex.data.Front() + entry, ok := e.Value.(*postIndexEntry) + if !ok { + return id, errors.New("NewestIndexedPostID: could not parse e as a postIndexEntry") + } + return entry.statusID, nil +} diff --git a/internal/timeline/index_test.go b/internal/timeline/index_test.go new file mode 100644 index 000000000..f48b2691c --- /dev/null +++ b/internal/timeline/index_test.go @@ -0,0 +1,193 @@ +/* + GoToSocial + Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see . +*/ + +package timeline_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/timeline" + "github.com/superseriousbusiness/gotosocial/testrig" +) + +type IndexTestSuite struct { + TimelineStandardTestSuite +} + +func (suite *IndexTestSuite) SetupSuite() { + suite.testAccounts = testrig.NewTestAccounts() + suite.testStatuses = testrig.NewTestStatuses() +} + +func (suite *IndexTestSuite) SetupTest() { + suite.config = testrig.NewTestConfig() + suite.db = testrig.NewTestDB() + suite.log = testrig.NewTestLog() + suite.tc = testrig.NewTestTypeConverter(suite.db) + + testrig.StandardDBSetup(suite.db, nil) + + // let's take local_account_1 as the timeline owner, and start with an empty timeline + tl, err := timeline.NewTimeline(suite.testAccounts["local_account_1"].ID, suite.db, suite.tc, suite.log) + if err != nil { + suite.FailNow(err.Error()) + } + suite.timeline = tl +} + +func (suite *IndexTestSuite) TearDownTest() { + testrig.StandardDBTeardown(suite.db) +} + +func (suite *IndexTestSuite) TestIndexBeforeLowID() { + // index 10 before the lowest status ID possible + err := suite.timeline.IndexBefore("00000000000000000000000000", true, 10) + suite.NoError(err) + + // the oldest indexed post should be the lowest one we have in our testrig + postID, err := suite.timeline.OldestIndexedPostID() + suite.NoError(err) + suite.Equal("01F8MHAAY43M6RJ473VQFCVH37", postID) + + // indexLength should only be 9 because that's all this user has hometimelineable + indexLength := suite.timeline.PostIndexLength() + suite.Equal(9, indexLength) +} + +func (suite *IndexTestSuite) TestIndexBeforeHighID() { + // index 10 before the highest status ID possible + err := suite.timeline.IndexBefore("ZZZZZZZZZZZZZZZZZZZZZZZZZZ", true, 10) + suite.NoError(err) + + // the oldest indexed post should be empty + postID, err := suite.timeline.OldestIndexedPostID() + suite.NoError(err) + suite.Empty(postID) + + // indexLength should be 0 + indexLength := suite.timeline.PostIndexLength() + suite.Equal(0, indexLength) +} + +func (suite *IndexTestSuite) TestIndexBehindHighID() { + // index 10 behind the highest status ID possible + err := suite.timeline.IndexBehind("ZZZZZZZZZZZZZZZZZZZZZZZZZZ", true, 10) + suite.NoError(err) + + // the newest indexed post should be the highest one we have in our testrig + postID, err := suite.timeline.NewestIndexedPostID() + suite.NoError(err) + suite.Equal("01FCTA44PW9H1TB328S9AQXKDS", postID) + + // indexLength should only be 11 because that's all this user has hometimelineable + indexLength := suite.timeline.PostIndexLength() + suite.Equal(11, indexLength) +} + +func (suite *IndexTestSuite) TestIndexBehindLowID() { + // index 10 behind the lowest status ID possible + err := suite.timeline.IndexBehind("00000000000000000000000000", true, 10) + suite.NoError(err) + + // the newest indexed post should be empty + postID, err := suite.timeline.NewestIndexedPostID() + suite.NoError(err) + suite.Empty(postID) + + // indexLength should be 0 + indexLength := suite.timeline.PostIndexLength() + suite.Equal(0, indexLength) +} + +func (suite *IndexTestSuite) TestOldestIndexedPostIDEmpty() { + // the oldest indexed post should be an empty string since there's nothing indexed yet + postID, err := suite.timeline.OldestIndexedPostID() + suite.NoError(err) + suite.Empty(postID) + + // indexLength should be 0 + indexLength := suite.timeline.PostIndexLength() + suite.Equal(0, indexLength) +} + +func (suite *IndexTestSuite) TestNewestIndexedPostIDEmpty() { + // the newest indexed post should be an empty string since there's nothing indexed yet + postID, err := suite.timeline.NewestIndexedPostID() + suite.NoError(err) + suite.Empty(postID) + + // indexLength should be 0 + indexLength := suite.timeline.PostIndexLength() + suite.Equal(0, indexLength) +} + +func (suite *IndexTestSuite) TestIndexAlreadyIndexed() { + testStatus := suite.testStatuses["local_account_1_status_1"] + + // index one post -- it should be indexed + indexed, err := suite.timeline.IndexOne(testStatus.CreatedAt, testStatus.ID, testStatus.BoostOfID, testStatus.AccountID, testStatus.BoostOfAccountID) + suite.NoError(err) + suite.True(indexed) + + // try to index the same post again -- it should not be indexed + indexed, err = suite.timeline.IndexOne(testStatus.CreatedAt, testStatus.ID, testStatus.BoostOfID, testStatus.AccountID, testStatus.BoostOfAccountID) + suite.NoError(err) + suite.False(indexed) +} + +func (suite *IndexTestSuite) TestIndexAndPrepareAlreadyIndexedAndPrepared() { + testStatus := suite.testStatuses["local_account_1_status_1"] + + // index and prepare one post -- it should be indexed + indexed, err := suite.timeline.IndexAndPrepareOne(testStatus.CreatedAt, testStatus.ID, testStatus.BoostOfID, testStatus.AccountID, testStatus.BoostOfAccountID) + suite.NoError(err) + suite.True(indexed) + + // try to index and prepare the same post again -- it should not be indexed + indexed, err = suite.timeline.IndexAndPrepareOne(testStatus.CreatedAt, testStatus.ID, testStatus.BoostOfID, testStatus.AccountID, testStatus.BoostOfAccountID) + suite.NoError(err) + suite.False(indexed) +} + +func (suite *IndexTestSuite) TestIndexBoostOfAlreadyIndexed() { + testStatus := suite.testStatuses["local_account_1_status_1"] + boostOfTestStatus := >smodel.Status{ + CreatedAt: time.Now(), + ID: "01FD4TA6G2Z6M7W8NJQ3K5WXYD", + BoostOfID: testStatus.ID, + AccountID: "01FD4TAY1C0NGEJVE9CCCX7QKS", + BoostOfAccountID: testStatus.AccountID, + } + + // index one post -- it should be indexed + indexed, err := suite.timeline.IndexOne(testStatus.CreatedAt, testStatus.ID, testStatus.BoostOfID, testStatus.AccountID, testStatus.BoostOfAccountID) + suite.NoError(err) + suite.True(indexed) + + // try to index the a boost of that post -- it should not be indexed + indexed, err = suite.timeline.IndexOne(boostOfTestStatus.CreatedAt, boostOfTestStatus.ID, boostOfTestStatus.BoostOfID, boostOfTestStatus.AccountID, boostOfTestStatus.BoostOfAccountID) + suite.NoError(err) + suite.False(indexed) +} + +func TestIndexTestSuite(t *testing.T) { + suite.Run(t, new(IndexTestSuite)) +} diff --git a/internal/timeline/manager.go b/internal/timeline/manager.go index 00d87bb26..a592670a8 100644 --- a/internal/timeline/manager.go +++ b/internal/timeline/manager.go @@ -75,11 +75,11 @@ type Manager interface { // PrepareXFromTop prepares limit n amount of posts, based on their indexed representations, from the top of the index. PrepareXFromTop(timelineAccountID string, limit int) error // Remove removes one status from the timeline of the given timelineAccountID - Remove(statusID string, timelineAccountID string) (int, error) + Remove(timelineAccountID string, statusID string) (int, error) // WipeStatusFromAllTimelines removes one status from the index and prepared posts of all timelines WipeStatusFromAllTimelines(statusID string) error // WipeStatusesFromAccountID removes all statuses by the given accountID from the timelineAccountID's timelines. - WipeStatusesFromAccountID(accountID string, timelineAccountID string) error + WipeStatusesFromAccountID(timelineAccountID string, accountID string) error } // NewManager returns a new timeline manager with the given database, typeconverter, config, and log. @@ -133,7 +133,7 @@ func (m *manager) IngestAndPrepare(status *gtsmodel.Status, timelineAccountID st return t.IndexAndPrepareOne(status.CreatedAt, status.ID, status.BoostOfID, status.AccountID, status.BoostOfAccountID) } -func (m *manager) Remove(statusID string, timelineAccountID string) (int, error) { +func (m *manager) Remove(timelineAccountID string, statusID string) (int, error) { l := m.log.WithFields(logrus.Fields{ "func": "Remove", "timelineAccountID": timelineAccountID, @@ -160,7 +160,7 @@ func (m *manager) HomeTimeline(timelineAccountID string, maxID string, sinceID s return nil, err } - statuses, err := t.Get(limit, maxID, sinceID, minID) + statuses, err := t.Get(limit, maxID, sinceID, minID, true) if err != nil { l.Errorf("error getting statuses: %s", err) } @@ -221,7 +221,7 @@ func (m *manager) WipeStatusFromAllTimelines(statusID string) error { return err } -func (m *manager) WipeStatusesFromAccountID(accountID string, timelineAccountID string) error { +func (m *manager) WipeStatusesFromAccountID(timelineAccountID string, accountID string) error { t, err := m.getOrCreateTimeline(timelineAccountID) if err != nil { return err diff --git a/internal/timeline/manager_test.go b/internal/timeline/manager_test.go new file mode 100644 index 000000000..9b975a5ce --- /dev/null +++ b/internal/timeline/manager_test.go @@ -0,0 +1,142 @@ +/* + GoToSocial + Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see . +*/ + +package timeline_test + +import ( + "testing" + + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/testrig" +) + +type ManagerTestSuite struct { + TimelineStandardTestSuite +} + +func (suite *ManagerTestSuite) SetupSuite() { + suite.testAccounts = testrig.NewTestAccounts() + suite.testStatuses = testrig.NewTestStatuses() +} + +func (suite *ManagerTestSuite) SetupTest() { + suite.config = testrig.NewTestConfig() + suite.db = testrig.NewTestDB() + suite.log = testrig.NewTestLog() + suite.tc = testrig.NewTestTypeConverter(suite.db) + + testrig.StandardDBSetup(suite.db, nil) + + manager := testrig.NewTestTimelineManager(suite.db) + suite.manager = manager +} + +func (suite *ManagerTestSuite) TearDownTest() { + testrig.StandardDBTeardown(suite.db) +} + +func (suite *ManagerTestSuite) TestManagerIntegration() { + testAccount := suite.testAccounts["local_account_1"] + + // should start at 0 + indexedLen := suite.manager.GetIndexedLength(testAccount.ID) + suite.Equal(0, indexedLen) + + // oldestIndexed should be empty string since there's nothing indexed + oldestIndexed, err := suite.manager.GetOldestIndexedID(testAccount.ID) + suite.NoError(err) + suite.Empty(oldestIndexed) + + // trigger status preparation + err = suite.manager.PrepareXFromTop(testAccount.ID, 20) + suite.NoError(err) + + // local_account_1 can see 11 statuses out of the testrig statuses in its home timeline + indexedLen = suite.manager.GetIndexedLength(testAccount.ID) + suite.Equal(11, indexedLen) + + // oldest should now be set + oldestIndexed, err = suite.manager.GetOldestIndexedID(testAccount.ID) + suite.NoError(err) + suite.Equal("01F8MH75CBF9JFX4ZAD54N0W0R", oldestIndexed) + + // get hometimeline + statuses, err := suite.manager.HomeTimeline(testAccount.ID, "", "", "", 20, false) + suite.NoError(err) + suite.Len(statuses, 11) + + // now wipe the last status from all timelines, as though it had been deleted by the owner + err = suite.manager.WipeStatusFromAllTimelines("01F8MH75CBF9JFX4ZAD54N0W0R") + suite.NoError(err) + + // timeline should be shorter + indexedLen = suite.manager.GetIndexedLength(testAccount.ID) + suite.Equal(10, indexedLen) + + // oldest should now be different + oldestIndexed, err = suite.manager.GetOldestIndexedID(testAccount.ID) + suite.NoError(err) + suite.Equal("01F8MH82FYRXD2RC6108DAJ5HB", oldestIndexed) + + // delete the new oldest status specifically from this timeline, as though local_account_1 had muted or blocked it + removed, err := suite.manager.Remove(testAccount.ID, "01F8MH82FYRXD2RC6108DAJ5HB") + suite.NoError(err) + suite.Equal(2, removed) // 1 status should be removed, but from both indexed and prepared, so 2 removals total + + // timeline should be shorter + indexedLen = suite.manager.GetIndexedLength(testAccount.ID) + suite.Equal(9, indexedLen) + + // oldest should now be different + oldestIndexed, err = suite.manager.GetOldestIndexedID(testAccount.ID) + suite.NoError(err) + suite.Equal("01F8MHAAY43M6RJ473VQFCVH37", oldestIndexed) + + // now remove all entries by local_account_2 from the timeline + err = suite.manager.WipeStatusesFromAccountID(testAccount.ID, suite.testAccounts["local_account_2"].ID) + suite.NoError(err) + + // timeline should be empty now + indexedLen = suite.manager.GetIndexedLength(testAccount.ID) + suite.Equal(5, indexedLen) + + // ingest 1 into the timeline + status1 := suite.testStatuses["admin_account_status_1"] + ingested, err := suite.manager.Ingest(status1, testAccount.ID) + suite.NoError(err) + suite.True(ingested) + + // ingest and prepare another one into the timeline + status2 := suite.testStatuses["local_account_2_status_1"] + ingested, err = suite.manager.IngestAndPrepare(status2, testAccount.ID) + suite.NoError(err) + suite.True(ingested) + + // timeline should be longer now + indexedLen = suite.manager.GetIndexedLength(testAccount.ID) + suite.Equal(7, indexedLen) + + // try to ingest status 2 again + ingested, err = suite.manager.IngestAndPrepare(status2, testAccount.ID) + suite.NoError(err) + suite.False(ingested) // should be false since it's a duplicate +} + +func TestManagerTestSuite(t *testing.T) { + suite.Run(t, new(ManagerTestSuite)) +} diff --git a/internal/timeline/prepare.go b/internal/timeline/prepare.go index 0fbd8ebba..51846c816 100644 --- a/internal/timeline/prepare.go +++ b/internal/timeline/prepare.go @@ -23,23 +23,35 @@ import ( "errors" "fmt" + "github.com/sirupsen/logrus" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) func (t *timeline) prepareNextQuery(amount int, maxID string, sinceID string, minID string) error { + l := t.log.WithFields(logrus.Fields{ + "func": "prepareNextQuery", + "amount": amount, + "maxID": maxID, + "sinceID": sinceID, + "minID": minID, + }) + var err error // maxID is defined but sinceID isn't so take from behind if maxID != "" && sinceID == "" { + l.Debug("preparing behind maxID") err = t.PrepareBehind(maxID, amount) } // maxID isn't defined, but sinceID || minID are, so take x before if maxID == "" && sinceID != "" { + l.Debug("preparing before sinceID") err = t.PrepareBefore(sinceID, false, amount) } if maxID == "" && minID != "" { + l.Debug("preparing before minID") err = t.PrepareBefore(minID, false, amount) } @@ -47,15 +59,16 @@ func (t *timeline) prepareNextQuery(amount int, maxID string, sinceID string, mi } func (t *timeline) PrepareBehind(statusID string, amount int) error { - t.Lock() - defer t.Unlock() - // lazily initialize prepared posts if it hasn't been done already if t.preparedPosts.data == nil { t.preparedPosts.data = &list.List{} t.preparedPosts.data.Init() } + if err := t.IndexBehind(statusID, true, amount); err != nil { + return fmt.Errorf("PrepareBehind: error indexing behind id %s: %s", statusID, err) + } + // if the postindex is nil, nothing has been indexed yet so there's nothing to prepare if t.postIndex.data == nil { return nil @@ -63,6 +76,8 @@ func (t *timeline) PrepareBehind(statusID string, amount int) error { var prepared int var preparing bool + t.Lock() + defer t.Unlock() prepareloop: for e := t.postIndex.data.Front(); e != nil; e = e.Next() { entry, ok := e.Value.(*postIndexEntry) @@ -154,8 +169,10 @@ prepareloop: } func (t *timeline) PrepareFromTop(amount int) error { - t.Lock() - defer t.Unlock() + l := t.log.WithFields(logrus.Fields{ + "func": "PrepareFromTop", + "amount": amount, + }) // lazily initialize prepared posts if it hasn't been done already if t.preparedPosts.data == nil { @@ -163,11 +180,17 @@ func (t *timeline) PrepareFromTop(amount int) error { t.preparedPosts.data.Init() } - // if the postindex is nil, nothing has been indexed yet so there's nothing to prepare + // if the postindex is nil, nothing has been indexed yet so index from the highest ID possible if t.postIndex.data == nil { - return nil + l.Debug("postindex.data was nil, indexing behind highest possible ID") + if err := t.IndexBehind("ZZZZZZZZZZZZZZZZZZZZZZZZZZ", false, amount); err != nil { + return fmt.Errorf("PrepareFromTop: error indexing behind id %s: %s", "ZZZZZZZZZZZZZZZZZZZZZZZZZZ", err) + } } + l.Trace("entering prepareloop") + t.Lock() + defer t.Unlock() var prepared int prepareloop: for e := t.postIndex.data.Front(); e != nil; e = e.Next() { @@ -193,10 +216,12 @@ prepareloop: prepared = prepared + 1 if prepared == amount { // we're done + l.Trace("leaving prepareloop") break prepareloop } } + l.Trace("leaving function") return nil } diff --git a/internal/timeline/timeline.go b/internal/timeline/timeline.go index 20015a745..6274a86ac 100644 --- a/internal/timeline/timeline.go +++ b/internal/timeline/timeline.go @@ -38,7 +38,10 @@ type Timeline interface { RETRIEVAL FUNCTIONS */ - Get(amount int, maxID string, sinceID string, minID string) ([]*apimodel.Status, error) + // Get returns an amount of statuses with the given parameters. + // If prepareNext is true, then the next predicted query will be prepared already in a goroutine, + // to make the next call to Get faster. + Get(amount int, maxID string, sinceID string, minID string, prepareNext bool) ([]*apimodel.Status, error) // GetXFromTop returns x amount of posts from the top of the timeline, from newest to oldest. GetXFromTop(amount int) ([]*apimodel.Status, error) // GetXBehindID returns x amount of posts from the given id onwards, from newest to oldest. @@ -50,7 +53,7 @@ type Timeline interface { // This will NOT include the status with the given ID. // // This corresponds to an api call to /timelines/home?since_id=WHATEVER - GetXBeforeID(amount int, sinceID string, startFromTop bool, attempts *int) ([]*apimodel.Status, error) + GetXBeforeID(amount int, sinceID string, startFromTop bool) ([]*apimodel.Status, error) // GetXBetweenID returns x amount of posts from the given maxID, up to the given id, from newest to oldest. // This will NOT include the status with the given IDs. // @@ -70,6 +73,12 @@ type Timeline interface { // OldestIndexedPostID returns the id of the rearmost (ie., the oldest) indexed post, or an error if something goes wrong. // If nothing goes wrong but there's no oldest post, an empty string will be returned so make sure to check for this. OldestIndexedPostID() (string, error) + // NewestIndexedPostID returns the id of the frontmost (ie., the newest) indexed post, or an error if something goes wrong. + // If nothing goes wrong but there's no newest post, an empty string will be returned so make sure to check for this. + NewestIndexedPostID() (string, error) + + IndexBefore(statusID string, include bool, amount int) error + IndexBehind(statusID string, include bool, amount int) error /* PREPARATION FUNCTIONS diff --git a/internal/timeline/timeline_test.go b/internal/timeline/timeline_test.go new file mode 100644 index 000000000..4f997cb1e --- /dev/null +++ b/internal/timeline/timeline_test.go @@ -0,0 +1,43 @@ +/* + GoToSocial + Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see . +*/ + +package timeline_test + +import ( + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/timeline" + "github.com/superseriousbusiness/gotosocial/internal/typeutils" +) + +type TimelineStandardTestSuite struct { + suite.Suite + config *config.Config + db db.DB + log *logrus.Logger + tc typeutils.TypeConverter + + testAccounts map[string]*gtsmodel.Account + testStatuses map[string]*gtsmodel.Status + + timeline timeline.Timeline + manager timeline.Manager +}