diff options
| -rw-r--r-- | internal/db/bundb/account.go | 5 | ||||
| -rw-r--r-- | internal/db/bundb/account_test.go | 106 | ||||
| -rw-r--r-- | internal/db/bundb/migrations/20240220204526_add_statuses_mentions_is_null_or_empty_idx.go | 82 | ||||
| -rw-r--r-- | internal/db/bundb/util.go | 20 |
4 files changed, 212 insertions, 1 deletions
diff --git a/internal/db/bundb/account.go b/internal/db/bundb/account.go index e0d574f62..d2c9c2f51 100644 --- a/internal/db/bundb/account.go +++ b/internal/db/bundb/account.go @@ -604,13 +604,16 @@ func (a *accountDB) GetAccountStatuses(ctx context.Context, accountID string, li Where("? = ?", bun.Ident("status.account_id"), accountID) if excludeReplies { - q = q.WhereGroup(" AND ", func(*bun.SelectQuery) *bun.SelectQuery { + q = q.WhereGroup(" AND ", func(q *bun.SelectQuery) *bun.SelectQuery { return q. // Do include self replies (threads), but // don't include replies to other people. Where("? = ?", bun.Ident("status.in_reply_to_account_id"), accountID). WhereOr("? IS NULL", bun.Ident("status.in_reply_to_uri")) }) + // Don't include replies that mention other people: + // for example, an account's reply to its own reply to someone else. + q = whereArrayIsNullOrEmpty(q, bun.Ident("status.mentions")) } if excludeReblogs { diff --git a/internal/db/bundb/account_test.go b/internal/db/bundb/account_test.go index d7f0c1c72..f01964da8 100644 --- a/internal/db/bundb/account_test.go +++ b/internal/db/bundb/account_test.go @@ -22,6 +22,7 @@ import ( "crypto/rand" "crypto/rsa" "errors" + "fmt" "reflect" "strings" "testing" @@ -32,6 +33,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/db/bundb" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/util" "github.com/uptrace/bun" ) @@ -85,6 +87,110 @@ func (suite *AccountTestSuite) TestGetAccountStatusesExcludeRepliesAndReblogsPub suite.Len(statuses, 2) } +// populateTestStatus adds mandatory fields to a partially populated status. +func (suite *AccountTestSuite) populateTestStatus(testAccountKey string, status *gtsmodel.Status, inReplyTo *gtsmodel.Status) *gtsmodel.Status { + testAccount := suite.testAccounts[testAccountKey] + if testAccount == nil { + suite.FailNowf("", "Missing test account: %s", testAccountKey) + return status + } + if testAccount.Domain != "" { + suite.FailNowf("", "Only local test accounts are supported: %s is remote", testAccountKey) + return status + } + + status.AccountID = testAccount.ID + status.AccountURI = testAccount.URI + status.URI = fmt.Sprintf("http://localhost:8080/users/%s/statuses/%s", testAccount.Username, status.ID) + status.Local = util.Ptr(true) + + if status.Visibility == "" { + status.Visibility = gtsmodel.VisibilityDefault + } + if status.ActivityStreamsType == "" { + status.ActivityStreamsType = ap.ObjectNote + } + if status.Federated == nil { + status.Federated = util.Ptr(true) + } + if status.Boostable == nil { + status.Boostable = util.Ptr(true) + } + if status.Likeable == nil { + status.Likeable = util.Ptr(true) + } + if status.Replyable == nil { + status.Replyable = util.Ptr(true) + } + + if inReplyTo != nil { + status.InReplyToAccountID = inReplyTo.AccountID + status.InReplyToID = inReplyTo.ID + status.InReplyToURI = inReplyTo.URI + } + + return status +} + +// Tests that we're including self-replies but excluding those that mention other accounts. +func (suite *AccountTestSuite) TestGetAccountStatusesExcludeRepliesExcludesSelfRepliesWithMentions() { + post := suite.populateTestStatus( + "local_account_1", + >smodel.Status{ + ID: "01HQ1FGN679M5F81DZ18WS6JQG", + Content: "post", + }, + nil, + ) + reply := suite.populateTestStatus( + "local_account_2", + >smodel.Status{ + ID: "01HQ1GTXMT2W6PF8MA2XG9DG6Q", + Content: "post", + MentionIDs: []string{post.InReplyToAccountID}, + }, + post, + ) + riposte := suite.populateTestStatus( + "local_account_1", + >smodel.Status{ + ID: "01HQ1GTXN0RWG9ZWJKRFAEF5RE", + Content: "riposte", + MentionIDs: []string{reply.InReplyToAccountID}, + }, + reply, + ) + followup := suite.populateTestStatus( + "local_account_1", + >smodel.Status{ + ID: "01HQ1GTXN52X7MM9Z12PNJWEHQ", + Content: "followup", + MentionIDs: []string{reply.InReplyToAccountID}, + }, + riposte, + ) + + for _, status := range []*gtsmodel.Status{post, reply, riposte, followup} { + if err := suite.db.PutStatus(context.Background(), status); err != nil { + suite.FailNowf("", "Error while adding test status with ID %s: %v", status.ID, err) + return + } + } + + testAccount := suite.testAccounts["local_account_1"] + statuses, err := suite.db.GetAccountStatuses(context.Background(), testAccount.ID, 20, true, true, "", "", false, false) + suite.NoError(err) + suite.Len(statuses, 8) + for _, status := range statuses { + if status.InReplyToID != "" && status.InReplyToAccountID != testAccount.ID { + suite.FailNowf("", "Status with ID %s is a non-self reply and should have been excluded", status.ID) + } + if len(status.Mentions) != 0 { + suite.FailNowf("", "Status with ID %s has mentions and should have been excluded", status.ID) + } + } +} + func (suite *AccountTestSuite) TestGetAccountStatusesMediaOnly() { statuses, err := suite.db.GetAccountStatuses(context.Background(), suite.testAccounts["local_account_1"].ID, 20, false, false, "", "", true, false) suite.NoError(err) diff --git a/internal/db/bundb/migrations/20240220204526_add_statuses_mentions_is_null_or_empty_idx.go b/internal/db/bundb/migrations/20240220204526_add_statuses_mentions_is_null_or_empty_idx.go new file mode 100644 index 000000000..632abd2df --- /dev/null +++ b/internal/db/bundb/migrations/20240220204526_add_statuses_mentions_is_null_or_empty_idx.go @@ -0,0 +1,82 @@ +// GoToSocial +// Copyright (C) GoToSocial Authors admin@gotosocial.org +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// 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 <http://www.gnu.org/licenses/>. + +package migrations + +import ( + "context" + + gtsmodel "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/uptrace/bun" + "github.com/uptrace/bun/dialect" +) + +func init() { + up := func(ctx context.Context, db *bun.DB) error { + var expression string + switch db.Dialect().Name() { + case dialect.PG: + expression = "(mentions IS NULL OR CARDINALITY(mentions) = 0)" + case dialect.SQLite: + expression = "(mentions IS NULL OR json_array_length(mentions) = 0)" + default: + panic("db conn was neither pg not sqlite") + } + + return db.RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error { + // Remove previous index for viewing + // statuses created by account. + if _, err := tx. + NewDropIndex(). + Index("statuses_account_id_id_idx"). + IfExists(). + Exec(ctx); err != nil { + return err + } + + // Add new index for viewing statuses created + // by account, which includes mentions in the index. + if _, err := tx. + NewCreateIndex(). + Model((*gtsmodel.Status)(nil)). + Index("statuses_account_view_idx"). + Column( + "account_id", + "in_reply_to_account_id", + "in_reply_to_uri", + ). + ColumnExpr(expression). + ColumnExpr("id DESC"). + IfNotExists(). + Exec(ctx); err != nil { + return err + } + + return nil + }) + } + + down := func(ctx context.Context, db *bun.DB) error { + return db.RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error { + return nil + }) + } + + if err := Migrations.Register(up, down); err != nil { + panic(err) + } +} diff --git a/internal/db/bundb/util.go b/internal/db/bundb/util.go index e2dd392dc..c7a2b96c2 100644 --- a/internal/db/bundb/util.go +++ b/internal/db/bundb/util.go @@ -206,3 +206,23 @@ func parseWhere(w db.Where) (query string, args []interface{}) { args = []interface{}{bun.Ident(w.Key), w.Value} return } + +// whereArrayIsNullOrEmpty extends a query with a where clause requiring an array to be null or empty. +// (The empty check varies by dialect; only PG has direct support for SQL array types.) +func whereArrayIsNullOrEmpty(query *bun.SelectQuery, subject interface{}) *bun.SelectQuery { + var arrayEmptySQL string + switch d := query.Dialect().Name(); d { + case dialect.SQLite: + arrayEmptySQL = "json_array_length(?) = 0" + case dialect.PG: + arrayEmptySQL = "CARDINALITY(?) = 0" + default: + log.Panicf(nil, "db conn %s was neither pg nor sqlite", d) + } + + return query.WhereGroup(" AND ", func(q *bun.SelectQuery) *bun.SelectQuery { + return q. + Where("? IS NULL", subject). + WhereOr(arrayEmptySQL, subject) + }) +} |