From f78f639e9fc574924a7639b464faae57bb513752 Mon Sep 17 00:00:00 2001 From: tsmethurst Date: Sat, 14 Aug 2021 14:33:18 +0200 Subject: [PATCH] fiddle with tests some more --- internal/timeline/get.go | 54 ++++++++----- internal/timeline/get_test.go | 140 +++++++++++++++++++++++++++++++--- internal/timeline/manager.go | 2 +- internal/timeline/timeline.go | 5 +- 4 files changed, 169 insertions(+), 32 deletions(-) diff --git a/internal/timeline/get.go b/internal/timeline/get.go index 72732efda..bf4c0dcd5 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, @@ -50,12 +50,14 @@ func (t *timeline) Get(amount int, maxID string, sinceID string, minID string) ( // 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 - // 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) - } - }() + 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) + } + }() + } } } @@ -67,12 +69,14 @@ func (t *timeline) Get(amount int, maxID string, sinceID string, minID string) ( // 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 - // 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) - } - }() + 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) + } + }() + } } } @@ -130,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 @@ -154,8 +165,8 @@ findMarkLoop: } if entry.statusID <= behindID { + l.Trace("found behindID mark") behindIDMark = e - } else { break findMarkLoop } } @@ -173,12 +184,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) } diff --git a/internal/timeline/get_test.go b/internal/timeline/get_test.go index af7929e23..9e1865c37 100644 --- a/internal/timeline/get_test.go +++ b/internal/timeline/get_test.go @@ -19,9 +19,7 @@ package timeline_test import ( - "fmt" "testing" - "time" "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/timeline" @@ -66,20 +64,138 @@ func (suite *GetTestSuite) TearDownTest() { testrig.StandardDBTeardown(suite.db) } -func (suite *GetTestSuite) TestGet() { - // get 10 from the top - statuses, err := suite.timeline.Get(10, "", "", "") +func (suite *GetTestSuite) TestGetDefault() { + // get 10 from the top and don't prepare the next query + statuses, err := suite.timeline.Get(10, "", "", "", false) if err != nil { suite.FailNow(err.Error()) } - fmt.Println(len(statuses)) - fmt.Println(len(statuses)) - fmt.Println(len(statuses)) - fmt.Println(len(statuses)) - fmt.Println(len(statuses)) - fmt.Println(len(statuses)) - time.Sleep(1 * time.Second) + 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 + } + } +} + +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 := 5 + 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 := 5 + 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 := 5 + 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 := 5 + 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 TestGetTestSuite(t *testing.T) { diff --git a/internal/timeline/manager.go b/internal/timeline/manager.go index 00d87bb26..e2a98f623 100644 --- a/internal/timeline/manager.go +++ b/internal/timeline/manager.go @@ -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) } diff --git a/internal/timeline/timeline.go b/internal/timeline/timeline.go index 0c749ec96..9ef32401a 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.