From 79ccd8fd8acd09f78385cda799e9049cbc59de0f Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Mon, 22 Nov 2021 14:40:23 +0100 Subject: [PATCH] Fix mentioned accounts visibility bug (#323) * update other tests * set test status to followers_only * add test dm * fix mentioned accounts not being added to relevantAccounts * add some visibility tests for statuses --- internal/db/bundb/basic_test.go | 2 +- internal/timeline/get_test.go | 16 +-- internal/timeline/index_test.go | 4 +- internal/timeline/manager_test.go | 10 +- internal/visibility/filter_test.go | 74 ++++++++++++++ internal/visibility/relevantaccounts.go | 1 + internal/visibility/statusvisible_test.go | 115 ++++++++++++++++++++++ testrig/testmodels.go | 40 +++++++- 8 files changed, 245 insertions(+), 17 deletions(-) create mode 100644 internal/visibility/filter_test.go create mode 100644 internal/visibility/statusvisible_test.go diff --git a/internal/db/bundb/basic_test.go b/internal/db/bundb/basic_test.go index e5f7e159a..3c9eb8a86 100644 --- a/internal/db/bundb/basic_test.go +++ b/internal/db/bundb/basic_test.go @@ -43,7 +43,7 @@ func (suite *BasicTestSuite) TestGetAllStatuses() { s := []*gtsmodel.Status{} err := suite.db.GetAll(context.Background(), &s) suite.NoError(err) - suite.Len(s, 13) + suite.Len(s, 14) } func (suite *BasicTestSuite) TestGetAllNotNull() { diff --git a/internal/timeline/get_test.go b/internal/timeline/get_test.go index 22b2c5be8..b05585cab 100644 --- a/internal/timeline/get_test.go +++ b/internal/timeline/get_test.go @@ -73,8 +73,8 @@ func (suite *GetTestSuite) TestGetDefault() { suite.FailNow(err.Error()) } - // we only have 13 statuses in the test suite - suite.Len(statuses, 13) + // we only have 14 statuses in the test suite + suite.Len(statuses, 14) // statuses should be sorted highest to lowest ID var highest string @@ -166,8 +166,8 @@ func (suite *GetTestSuite) TestGetMinID() { suite.FailNow(err.Error()) } - // we should only get 6 statuses back, since we asked for a min ID that excludes some of our entries - suite.Len(statuses, 6) + // we should only get 7 statuses back, since we asked for a min ID that excludes some of our entries + suite.Len(statuses, 7) // statuses should be sorted highest to lowest ID var highest string @@ -188,8 +188,8 @@ func (suite *GetTestSuite) TestGetSinceID() { suite.FailNow(err.Error()) } - // we should only get 6 statuses back, since we asked for a since ID that excludes some of our entries - suite.Len(statuses, 6) + // we should only get 7 statuses back, since we asked for a since ID that excludes some of our entries + suite.Len(statuses, 7) // statuses should be sorted highest to lowest ID var highest string @@ -210,8 +210,8 @@ func (suite *GetTestSuite) TestGetSinceIDPrepareNext() { suite.FailNow(err.Error()) } - // we should only get 6 statuses back, since we asked for a since ID that excludes some of our entries - suite.Len(statuses, 6) + // we should only get 7 statuses back, since we asked for a since ID that excludes some of our entries + suite.Len(statuses, 7) // statuses should be sorted highest to lowest ID var highest string diff --git a/internal/timeline/index_test.go b/internal/timeline/index_test.go index 681f7617b..0f3a1df4a 100644 --- a/internal/timeline/index_test.go +++ b/internal/timeline/index_test.go @@ -66,7 +66,7 @@ func (suite *IndexTestSuite) TestIndexBeforeLowID() { // the oldest indexed post should be the lowest one we have in our testrig postID, err := suite.timeline.OldestIndexedPostID(context.Background()) suite.NoError(err) - suite.Equal("01F8MHAMCHF6Y650WCRSCP4WMY", postID) + suite.Equal("01F8MHAYFKS4KMXF8K5Y1C0KRN", postID) indexLength := suite.timeline.PostIndexLength(context.Background()) suite.Equal(10, indexLength) @@ -95,7 +95,7 @@ func (suite *IndexTestSuite) TestIndexBehindHighID() { // the newest indexed post should be the highest one we have in our testrig postID, err := suite.timeline.NewestIndexedPostID(context.Background()) suite.NoError(err) - suite.Equal("01FF25D5Q0DH7CHD57CTRS6WK0", postID) + suite.Equal("01FN3VJGFH10KR7S2PB0GFJZYG", postID) // indexLength should be 10 because that's all this user has hometimelineable indexLength := suite.timeline.PostIndexLength(context.Background()) diff --git a/internal/timeline/manager_test.go b/internal/timeline/manager_test.go index 072761cd7..4d725e48d 100644 --- a/internal/timeline/manager_test.go +++ b/internal/timeline/manager_test.go @@ -67,9 +67,9 @@ func (suite *ManagerTestSuite) TestManagerIntegration() { err = suite.manager.PrepareXFromTop(context.Background(), testAccount.ID, 20) suite.NoError(err) - // local_account_1 can see 13 statuses out of the testrig statuses in its home timeline + // local_account_1 can see 14 statuses out of the testrig statuses in its home timeline indexedLen = suite.manager.GetIndexedLength(context.Background(), testAccount.ID) - suite.Equal(13, indexedLen) + suite.Equal(14, indexedLen) // oldest should now be set oldestIndexed, err = suite.manager.GetOldestIndexedID(context.Background(), testAccount.ID) @@ -79,7 +79,7 @@ func (suite *ManagerTestSuite) TestManagerIntegration() { // get hometimeline statuses, err := suite.manager.HomeTimeline(context.Background(), testAccount.ID, "", "", "", 20, false) suite.NoError(err) - suite.Len(statuses, 13) + suite.Len(statuses, 14) // now wipe the last status from all timelines, as though it had been deleted by the owner err = suite.manager.WipeStatusFromAllTimelines(context.Background(), "01F8MH75CBF9JFX4ZAD54N0W0R") @@ -87,7 +87,7 @@ func (suite *ManagerTestSuite) TestManagerIntegration() { // timeline should be shorter indexedLen = suite.manager.GetIndexedLength(context.Background(), testAccount.ID) - suite.Equal(12, indexedLen) + suite.Equal(13, indexedLen) // oldest should now be different oldestIndexed, err = suite.manager.GetOldestIndexedID(context.Background(), testAccount.ID) @@ -101,7 +101,7 @@ func (suite *ManagerTestSuite) TestManagerIntegration() { // timeline should be shorter indexedLen = suite.manager.GetIndexedLength(context.Background(), testAccount.ID) - suite.Equal(11, indexedLen) + suite.Equal(12, indexedLen) // oldest should now be different oldestIndexed, err = suite.manager.GetOldestIndexedID(context.Background(), testAccount.ID) diff --git a/internal/visibility/filter_test.go b/internal/visibility/filter_test.go new file mode 100644 index 000000000..a140d48e2 --- /dev/null +++ b/internal/visibility/filter_test.go @@ -0,0 +1,74 @@ +/* + 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 visibility_test + +import ( + "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/visibility" + "github.com/superseriousbusiness/gotosocial/testrig" +) + +type FilterStandardTestSuite struct { + // standard suite interfaces + suite.Suite + config *config.Config + db db.DB + + // standard suite models + testTokens map[string]*gtsmodel.Token + testClients map[string]*gtsmodel.Client + testApplications map[string]*gtsmodel.Application + testUsers map[string]*gtsmodel.User + testAccounts map[string]*gtsmodel.Account + testAttachments map[string]*gtsmodel.MediaAttachment + testStatuses map[string]*gtsmodel.Status + testTags map[string]*gtsmodel.Tag + testMentions map[string]*gtsmodel.Mention + + filter visibility.Filter +} + +func (suite *FilterStandardTestSuite) SetupSuite() { + suite.testTokens = testrig.NewTestTokens() + suite.testClients = testrig.NewTestClients() + suite.testApplications = testrig.NewTestApplications() + suite.testUsers = testrig.NewTestUsers() + suite.testAccounts = testrig.NewTestAccounts() + suite.testAttachments = testrig.NewTestAttachments() + suite.testStatuses = testrig.NewTestStatuses() + suite.testTags = testrig.NewTestTags() + suite.testMentions = testrig.NewTestMentions() +} + +func (suite *FilterStandardTestSuite) SetupTest() { + testrig.InitTestLog() + + suite.config = testrig.NewTestConfig() + suite.db = testrig.NewTestDB() + suite.filter = visibility.NewFilter(suite.db) + + testrig.StandardDBSetup(suite.db, nil) +} + +func (suite *FilterStandardTestSuite) TearDownTest() { + testrig.StandardDBTeardown(suite.db) +} diff --git a/internal/visibility/relevantaccounts.go b/internal/visibility/relevantaccounts.go index d19d26ff4..1be5efea9 100644 --- a/internal/visibility/relevantaccounts.go +++ b/internal/visibility/relevantaccounts.go @@ -134,6 +134,7 @@ func (f *filter) relevantAccounts(ctx context.Context, status *gtsmodel.Status, } if mentionIn(m, status.MentionIDs) { nm = append(nm, m) + relAccts.MentionedAccounts = append(relAccts.MentionedAccounts, m.TargetAccount) } } status.Mentions = nm diff --git a/internal/visibility/statusvisible_test.go b/internal/visibility/statusvisible_test.go new file mode 100644 index 000000000..f92bcd62f --- /dev/null +++ b/internal/visibility/statusvisible_test.go @@ -0,0 +1,115 @@ +/* + 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 visibility_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/suite" +) + +type StatusVisibleTestSuite struct { + FilterStandardTestSuite +} + +func (suite *StatusVisibleTestSuite) TestOwnStatusVisible() { + testStatus := suite.testStatuses["local_account_1_status_1"] + testAccount := suite.testAccounts["local_account_1"] + ctx := context.Background() + + visible, err := suite.filter.StatusVisible(ctx, testStatus, testAccount) + suite.NoError(err) + + suite.True(visible) +} + +func (suite *StatusVisibleTestSuite) TestOwnDMVisible() { + ctx := context.Background() + + testStatusID := suite.testStatuses["local_account_2_status_6"].ID + testStatus, err := suite.db.GetStatusByID(ctx, testStatusID) + suite.NoError(err) + testAccount := suite.testAccounts["local_account_2"] + + visible, err := suite.filter.StatusVisible(ctx, testStatus, testAccount) + suite.NoError(err) + + suite.True(visible) +} + +func (suite *StatusVisibleTestSuite) TestDMVisibleToTarget() { + ctx := context.Background() + + testStatusID := suite.testStatuses["local_account_2_status_6"].ID + testStatus, err := suite.db.GetStatusByID(ctx, testStatusID) + suite.NoError(err) + testAccount := suite.testAccounts["local_account_1"] + + visible, err := suite.filter.StatusVisible(ctx, testStatus, testAccount) + suite.NoError(err) + + suite.True(visible) +} + +func (suite *StatusVisibleTestSuite) TestDMNotVisibleIfNotMentioned() { + ctx := context.Background() + + testStatusID := suite.testStatuses["local_account_2_status_6"].ID + testStatus, err := suite.db.GetStatusByID(ctx, testStatusID) + suite.NoError(err) + testAccount := suite.testAccounts["admin_account"] + + visible, err := suite.filter.StatusVisible(ctx, testStatus, testAccount) + suite.NoError(err) + + suite.False(visible) +} + +func (suite *StatusVisibleTestSuite) TestStatusNotVisibleIfNotMutuals() { + ctx := context.Background() + + testStatusID := suite.testStatuses["local_account_1_status_4"].ID + testStatus, err := suite.db.GetStatusByID(ctx, testStatusID) + suite.NoError(err) + testAccount := suite.testAccounts["local_account_2"] + + visible, err := suite.filter.StatusVisible(ctx, testStatus, testAccount) + suite.NoError(err) + + suite.False(visible) +} + +func (suite *StatusVisibleTestSuite) TestStatusNotVisibleIfNotFollowing() { + ctx := context.Background() + + testStatusID := suite.testStatuses["local_account_1_status_5"].ID + testStatus, err := suite.db.GetStatusByID(ctx, testStatusID) + suite.NoError(err) + testAccount := suite.testAccounts["admin_account"] + + visible, err := suite.filter.StatusVisible(ctx, testStatus, testAccount) + suite.NoError(err) + + suite.False(visible) +} + +func TestStatusVisibleTestSuite(t *testing.T) { + suite.Run(t, new(StatusVisibleTestSuite)) +} diff --git a/testrig/testmodels.go b/testrig/testmodels.go index fc0047f20..5d7df1c77 100644 --- a/testrig/testmodels.go +++ b/testrig/testmodels.go @@ -1037,7 +1037,7 @@ func NewTestStatuses() map[string]*gtsmodel.Status { InReplyToID: "", BoostOfID: "", ContentWarning: "", - Visibility: gtsmodel.VisibilityMutualsOnly, + Visibility: gtsmodel.VisibilityFollowersOnly, Sensitive: false, Language: "en", CreatedWithApplicationID: "01F8MGY43H3N2C8EWPR2FPYEXG", @@ -1166,6 +1166,32 @@ func NewTestStatuses() map[string]*gtsmodel.Status { Likeable: true, ActivityStreamsType: ap.ObjectNote, }, + "local_account_2_status_6": { + ID: "01FN3VJGFH10KR7S2PB0GFJZYG", + URI: "http://localhost:8080/users/1happyturtle/statuses/01FN3VJGFH10KR7S2PB0GFJZYG", + URL: "http://localhost:8080/@1happyturtle/statuses/01FN3VJGFH10KR7S2PB0GFJZYG", + Content: "🐢 @the_mighty_zork hi zork, this is a direct message, shhhhhh! 🐢", + CreatedAt: time.Now().Add(-1 * time.Minute), + UpdatedAt: time.Now().Add(-1 * time.Minute), + Local: true, + AccountURI: "http://localhost:8080/users/1happyturtle", + MentionIDs: []string{"01FDF2HM2NF6FSRZCDEDV451CN"}, + AccountID: "01F8MH5NBDF2MV7CTC4Q5128HF", + InReplyToID: "", + InReplyToAccountID: "", + InReplyToURI: "", + BoostOfID: "", + ContentWarning: "", + Visibility: gtsmodel.VisibilityDirect, + Sensitive: false, + Language: "en", + CreatedWithApplicationID: "01F8MGYG9E893WRHW0TAEXR8GJ", + Federated: true, + Boostable: true, + Replyable: true, + Likeable: true, + ActivityStreamsType: ap.ObjectNote, + }, } } @@ -1224,6 +1250,18 @@ func NewTestMentions() map[string]*gtsmodel.Mention { TargetAccountURI: "http://localhost:8080/users/the_mighty_zork", TargetAccountURL: "http://localhost:8080/@the_mighty_zork", }, + "local_user_2_mention_zork_direct_message": { + ID: "01FN3VKDEF4CN2W9TKX339BEHB", + StatusID: "01FN3VJGFH10KR7S2PB0GFJZYG", + CreatedAt: time.Now().Add(-1 * time.Minute), + UpdatedAt: time.Now().Add(-1 * time.Minute), + OriginAccountID: "01F8MH5NBDF2MV7CTC4Q5128HF", + OriginAccountURI: "http://localhost:8080/users/1happyturtle", + TargetAccountID: "01F8MH1H7YV1Z7D2C8K2730QBF", + NameString: "@the_mighty_zork", + TargetAccountURI: "http://localhost:8080/users/the_mighty_zork", + TargetAccountURL: "http://localhost:8080/@the_mighty_zork", + }, "admin_account_mention_zork": { ID: "01FF26A6BGEKCZFWNEHXB2ZZ6M", StatusID: "01FF25D5Q0DH7CHD57CTRS6WK0",