[bugfix] Refactor parse mention, fix local mention bug (#2657)

* [bugfix] Refactor parse mention, fix local mention bug

* originAccount -> originAcct

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)

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")
 }

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 <http://www.gnu.org/licenses/>.
+
+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 &gtsmodel.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
+	}
+}

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 <http://www.gnu.org/licenses/>.
+
+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{})
+}

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)
 	)
 

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")

internal/processing/util.go

@@ -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 <http://www.gnu.org/licenses/>.
-
-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 &gtsmodel.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
-	}
-}

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)