From a314d2b539804a31e602774196676e2d0089b8dc Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Mon, 19 Feb 2024 04:48:20 +0100 Subject: [PATCH] [bugfix] Refactor parse mention, fix local mention bug (#2657) * [bugfix] Refactor parse mention, fix local mention bug * originAccount -> originAcct --- internal/gtsmodel/mention.go | 4 +- internal/processing/account/account_test.go | 2 +- internal/processing/parsemention.go | 119 ++++++++++++++++++++ internal/processing/parsemention_test.go | 108 ++++++++++++++++++ internal/processing/processor.go | 2 +- internal/processing/status/status_test.go | 2 +- internal/processing/util.go | 91 --------------- internal/text/formatter_test.go | 2 +- 8 files changed, 233 insertions(+), 97 deletions(-) create mode 100644 internal/processing/parsemention.go create mode 100644 internal/processing/parsemention_test.go delete mode 100644 internal/processing/util.go diff --git a/internal/gtsmodel/mention.go b/internal/gtsmodel/mention.go index fb7f3b51a..7d5516a9f 100644 --- a/internal/gtsmodel/mention.go +++ b/internal/gtsmodel/mention.go @@ -60,7 +60,7 @@ type Mention struct { // A pointer to the gtsmodel account of the mentioned account. } -// ParseMentionFunc describes a function that takes a lowercase account name +// ParseMentionFunc describes a function that takes a lowercase account namestring // in the form "@test@whatever.example.org" for a remote account, or "@test" // for a local account, and returns a fully populated mention for that account, // with the given origin status ID and origin account ID. @@ -70,4 +70,4 @@ type Mention struct { // // Mentions generated by this function are not put in the database, that's still up to // the caller to do. -type ParseMentionFunc func(ctx context.Context, targetAccount string, originAccountID string, statusID string) (*Mention, error) +type ParseMentionFunc func(ctx context.Context, namestring string, originAccountID string, statusID string) (*Mention, error) diff --git a/internal/processing/account/account_test.go b/internal/processing/account/account_test.go index d43b4c937..2eea5f438 100644 --- a/internal/processing/account/account_test.go +++ b/internal/processing/account/account_test.go @@ -115,7 +115,7 @@ func (suite *AccountStandardTestSuite) SetupTest() { filter := visibility.NewFilter(&suite.state) common := common.New(&suite.state, suite.tc, suite.federator, filter) - suite.accountProcessor = account.New(&common, &suite.state, suite.tc, suite.mediaManager, suite.oauthServer, suite.federator, filter, processing.GetParseMentionFunc(suite.db, suite.federator)) + suite.accountProcessor = account.New(&common, &suite.state, suite.tc, suite.mediaManager, suite.oauthServer, suite.federator, filter, processing.GetParseMentionFunc(&suite.state, suite.federator)) testrig.StandardDBSetup(suite.db, nil) testrig.StandardStorageSetup(suite.storage, "../../../testrig/media") } diff --git a/internal/processing/parsemention.go b/internal/processing/parsemention.go new file mode 100644 index 000000000..035f057b8 --- /dev/null +++ b/internal/processing/parsemention.go @@ -0,0 +1,119 @@ +// 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 . + +package processing + +import ( + "context" + "fmt" + + "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/federation" + "github.com/superseriousbusiness/gotosocial/internal/gtscontext" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/id" + "github.com/superseriousbusiness/gotosocial/internal/state" + "github.com/superseriousbusiness/gotosocial/internal/util" +) + +// GetParseMentionFunc returns a new ParseMentionFunc using the provided state and federator. +// State is used for doing local database lookups; federator is used for remote account lookups (if necessary). +func GetParseMentionFunc(state *state.State, federator *federation.Federator) gtsmodel.ParseMentionFunc { + return func(ctx context.Context, namestring string, originAccountID string, statusID string) (*gtsmodel.Mention, error) { + // Get the origin account first since + // we'll need it to create the mention. + originAcct, err := state.DB.GetAccountByID(ctx, originAccountID) + if err != nil { + return nil, fmt.Errorf( + "db error getting mention origin account %s: %w", + originAccountID, err, + ) + } + + // Parse target components from the + // "@someone@example.org" namestring. + targetUsername, targetHost, err := util.ExtractNamestringParts(namestring) + if err != nil { + return nil, fmt.Errorf( + "error extracting mention target: %w", + err, + ) + } + + // It's a "local" mention if namestring + // looks like one of the following: + // + // - "@someone" with no host component. + // - "@someone@gts.example.org" and we're host "gts.example.org". + // - "@someone@example.org" and we're account-domain "example.org". + local := targetHost == "" || + targetHost == config.GetHost() || + targetHost == config.GetAccountDomain() + + // Either a local or remote + // target for the mention. + var targetAcct *gtsmodel.Account + if local { + // Lookup local target accounts in the db only. + targetAcct, err = state.DB.GetAccountByUsernameDomain(ctx, targetUsername, "") + if err != nil { + return nil, fmt.Errorf( + "db error getting mention local target account %s: %w", + targetUsername, err, + ) + } + } else { + // If origin account is local, use + // it to do potential dereference. + // Else fallback to empty string, + // which uses instance account. + var requestUser string + if originAcct.IsLocal() { + requestUser = originAcct.Username + } + + targetAcct, _, err = federator.GetAccountByUsernameDomain( + gtscontext.SetFastFail(ctx), + requestUser, + targetUsername, + targetHost, + ) + if err != nil { + return nil, fmt.Errorf( + "error fetching mention remote target account: %w", + err, + ) + } + } + + // Return mention with useful populated fields, + // but *don't* store it in the database; that's + // up to the calling function to do, if they want. + return >smodel.Mention{ + ID: id.NewULID(), + StatusID: statusID, + OriginAccountID: originAcct.ID, + OriginAccountURI: originAcct.URI, + OriginAccount: originAcct, + TargetAccountID: targetAcct.ID, + TargetAccountURI: targetAcct.URI, + TargetAccountURL: targetAcct.URL, + TargetAccount: targetAcct, + NameString: namestring, + }, nil + } +} diff --git a/internal/processing/parsemention_test.go b/internal/processing/parsemention_test.go new file mode 100644 index 000000000..f97133969 --- /dev/null +++ b/internal/processing/parsemention_test.go @@ -0,0 +1,108 @@ +// 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 . + +package processing_test + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/id" + "github.com/superseriousbusiness/gotosocial/internal/processing" +) + +type ParseMentionTestSuite struct { + ProcessingStandardTestSuite +} + +func (suite *ParseMentionTestSuite) TestParseMentionFunc() { + var ( + ctx = context.Background() + parseMention = processing.GetParseMentionFunc(&suite.state, suite.federator) + originAcctID = suite.testAccounts["local_account_1"].ID + statusID = id.NewULID() + ) + + type testStruct struct { + namestring string + expectedTargetAcct *gtsmodel.Account + err error + } + + for i, test := range []testStruct{ + { + namestring: "@1happyturtle", + expectedTargetAcct: suite.testAccounts["local_account_2"], + }, + { + namestring: "@1happyturtle@localhost:8080", + expectedTargetAcct: suite.testAccounts["local_account_2"], + }, + { + namestring: "@foss_satan@fossbros-anonymous.io", + expectedTargetAcct: suite.testAccounts["remote_account_1"], + }, + { + namestring: "@foss_satan", + err: errors.New("db error getting mention local target account foss_satan: sql: no rows in result set"), + }, + { + namestring: "@foss_satan@aaaaaaaaaaaaaaaaaaa.example.org", + err: errors.New("error fetching mention remote target account: enrichAccount: error webfingering account: fingerRemoteAccount: error webfingering @foss_satan@aaaaaaaaaaaaaaaaaaa.example.org: failed to discover webfinger URL fallback for: aaaaaaaaaaaaaaaaaaa.example.org through host-meta: GET request for https://aaaaaaaaaaaaaaaaaaa.example.org/.well-known/host-meta failed: "), + }, + { + namestring: "pee pee poo poo", + err: errors.New("error extracting mention target: couldn't match namestring pee pee poo poo"), + }, + } { + mention, err := parseMention(ctx, test.namestring, originAcctID, statusID) + if test.err != nil { + suite.EqualError(err, test.err.Error()) + continue + } + + if err != nil { + suite.Fail(err.Error()) + continue + } + + if mention.OriginAccount == nil { + suite.Failf("nil origin account", "test %d, namestring %s", i+1, test.namestring) + continue + } + + if mention.TargetAccount == nil { + suite.Failf("nil target account", "test %d, namestring %s", i+1, test.namestring) + continue + } + + suite.NotEmpty(mention.ID) + suite.Equal(originAcctID, mention.OriginAccountID) + suite.Equal(originAcctID, mention.OriginAccount.ID) + suite.Equal(test.expectedTargetAcct.ID, mention.TargetAccountID) + suite.Equal(test.expectedTargetAcct.ID, mention.TargetAccount.ID) + suite.Equal(test.expectedTargetAcct.URI, mention.TargetAccountURI) + suite.Equal(test.expectedTargetAcct.URL, mention.TargetAccountURL) + } +} + +func TestParseMentionTestSuite(t *testing.T) { + suite.Run(t, &ParseMentionTestSuite{}) +} diff --git a/internal/processing/processor.go b/internal/processing/processor.go index 6ac7a6bf0..d4ef41bea 100644 --- a/internal/processing/processor.go +++ b/internal/processing/processor.go @@ -151,7 +151,7 @@ func NewProcessor( emailSender email.Sender, ) *Processor { var ( - parseMentionFunc = GetParseMentionFunc(state.DB, federator) + parseMentionFunc = GetParseMentionFunc(state, federator) filter = visibility.NewFilter(state) ) diff --git a/internal/processing/status/status_test.go b/internal/processing/status/status_test.go index 2a47a205e..37c7d6147 100644 --- a/internal/processing/status/status_test.go +++ b/internal/processing/status/status_test.go @@ -98,7 +98,7 @@ func (suite *StatusStandardTestSuite) SetupTest() { common := common.New(&suite.state, suite.typeConverter, suite.federator, filter) polls := polls.New(&common, &suite.state, suite.typeConverter) - suite.status = status.New(&suite.state, &common, &polls, suite.federator, suite.typeConverter, filter, processing.GetParseMentionFunc(suite.db, suite.federator)) + suite.status = status.New(&suite.state, &common, &polls, suite.federator, suite.typeConverter, filter, processing.GetParseMentionFunc(&suite.state, suite.federator)) testrig.StandardDBSetup(suite.db, suite.testAccounts) testrig.StandardStorageSetup(suite.storage, "../../../testrig/media") diff --git a/internal/processing/util.go b/internal/processing/util.go deleted file mode 100644 index 1ce67c616..000000000 --- a/internal/processing/util.go +++ /dev/null @@ -1,91 +0,0 @@ -// 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 . - -package processing - -import ( - "context" - "fmt" - - "github.com/superseriousbusiness/gotosocial/internal/config" - "github.com/superseriousbusiness/gotosocial/internal/db" - "github.com/superseriousbusiness/gotosocial/internal/federation" - "github.com/superseriousbusiness/gotosocial/internal/gtscontext" - "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" - "github.com/superseriousbusiness/gotosocial/internal/id" - "github.com/superseriousbusiness/gotosocial/internal/util" -) - -func GetParseMentionFunc(dbConn db.DB, federator *federation.Federator) gtsmodel.ParseMentionFunc { - return func(ctx context.Context, targetAccount string, originAccountID string, statusID string) (*gtsmodel.Mention, error) { - // get the origin account first since we'll need it to create the mention - originAccount, err := dbConn.GetAccountByID(ctx, originAccountID) - if err != nil { - return nil, fmt.Errorf("couldn't get mention origin account with id %s", originAccountID) - } - - username, domain, err := util.ExtractNamestringParts(targetAccount) - if err != nil { - return nil, fmt.Errorf("couldn't extract namestring parts from %s: %s", targetAccount, err) - } - - var mentionedAccount *gtsmodel.Account - if domain == "" || domain == config.GetHost() || domain == config.GetAccountDomain() { - localAccount, err := dbConn.GetAccountByUsernameDomain(ctx, username, "") - if err != nil { - return nil, err - } - mentionedAccount = localAccount - } else { - var requestingUsername string - - if originAccount.Domain == "" { - requestingUsername = originAccount.Username - } - - remoteAccount, _, err := federator.GetAccountByUsernameDomain( - gtscontext.SetFastFail(ctx), - requestingUsername, - username, - domain, - ) - if err != nil { - return nil, fmt.Errorf("parseMentionFunc: error fetching account: %s", err) - } - - // we were able to resolve it! - mentionedAccount = remoteAccount - } - - mentionID, err := id.NewRandomULID() - if err != nil { - return nil, err - } - - return >smodel.Mention{ - ID: mentionID, - StatusID: statusID, - OriginAccountID: originAccount.ID, - OriginAccountURI: originAccount.URI, - TargetAccountID: mentionedAccount.ID, - NameString: targetAccount, - TargetAccountURI: mentionedAccount.URI, - TargetAccountURL: mentionedAccount.URL, - OriginAccount: mentionedAccount, - }, nil - } -} diff --git a/internal/text/formatter_test.go b/internal/text/formatter_test.go index cce9970b2..6fd8f4d7b 100644 --- a/internal/text/formatter_test.go +++ b/internal/text/formatter_test.go @@ -74,7 +74,7 @@ func (suite *TextStandardTestSuite) SetupTest() { suite.db = testrig.NewTestDB(&state) federator := testrig.NewTestFederator(&state, testrig.NewTestTransportController(&state, testrig.NewMockHTTPClient(nil, "../../testrig/media")), nil) - suite.parseMention = processing.GetParseMentionFunc(suite.db, federator) + suite.parseMention = processing.GetParseMentionFunc(&state, federator) suite.formatter = text.NewFormatter(suite.db)