[chore] Migrate accounts to new table, relax uniqueness constraint of actor `url` and collections (#3928)

* [chore] Migrate accounts to new table, relax uniqueness constraint of actor url and collections

* fiddle with it! (that's what she said)

* remove unused cache fields

* sillyness

* fix tiny whoopsie

3 files changed, 97 insertions, 28 deletions

diff --git a/internal/federation/dereferencing/account.go b/internal/federation/dereferencing/account.go
index 77de954c8..f882eb7c3 100644
--- a/internal/federation/dereferencing/account.go
+++ b/internal/federation/dereferencing/account.go
@@ -24,6 +24,7 @@ import (
 	"net/url"
 	"time"
 
+	errorsv2 "codeberg.org/gruf/go-errors/v2"
 	"codeberg.org/superseriousbusiness/activity/pub"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
 	"github.com/superseriousbusiness/gotosocial/internal/config"
@@ -88,14 +89,30 @@ func accountFresh(
 	return !time.Now().After(staleAt)
 }
 
-// GetAccountByURI will attempt to fetch an accounts by its URI, first checking the database. In the case of a newly-met remote model, or a remote model
-// whose last_fetched date is beyond a certain interval, the account will be dereferenced. In the case of dereferencing, some low-priority account information
-// may be enqueued for asynchronous fetching, e.g. featured account statuses (pins). An ActivityPub object indicates the account was dereferenced.
-func (d *Dereferencer) GetAccountByURI(ctx context.Context, requestUser string, uri *url.URL) (*gtsmodel.Account, ap.Accountable, error) {
+// GetAccountByURI will attempt to fetch an accounts by its
+// URI, first checking the database. In the case of a newly-met
+// remote model, or a remote model whose last_fetched date is
+// beyond a certain interval, the account will be dereferenced.
+// In the case of dereferencing, some low-priority account info
+// may be enqueued for asynchronous fetching, e.g. pinned statuses.
+// An ActivityPub object indicates the account was dereferenced.
+//
+// if tryURL is true, then the database will also check for a *single*
+// account where uri == account.url, not just uri == account.uri.
+// Because url does not guarantee uniqueness, you should only set
+// tryURL to true when doing searches set in motion by a user,
+// ie., when it's not important that an exact account is returned.
+func (d *Dereferencer) GetAccountByURI(
+	ctx context.Context,
+	requestUser string,
+	uri *url.URL,
+	tryURL bool,
+) (*gtsmodel.Account, ap.Accountable, error) {
 	// Fetch and dereference account if necessary.
 	account, accountable, err := d.getAccountByURI(ctx,
 		requestUser,
 		uri,
+		tryURL,
 	)
 	if err != nil {
 		return nil, nil, err
@@ -117,8 +134,15 @@ func (d *Dereferencer) GetAccountByURI(ctx context.Context, requestUser string,
 	return account, accountable, nil
 }
 
-// getAccountByURI is a package internal form of .GetAccountByURI() that doesn't bother dereferencing featured posts on update.
-func (d *Dereferencer) getAccountByURI(ctx context.Context, requestUser string, uri *url.URL) (*gtsmodel.Account, ap.Accountable, error) {
+// getAccountByURI is a package internal form of
+// .GetAccountByURI() that doesn't bother dereferencing
+// featured posts on update.
+func (d *Dereferencer) getAccountByURI(
+	ctx context.Context,
+	requestUser string,
+	uri *url.URL,
+	tryURL bool,
+) (*gtsmodel.Account, ap.Accountable, error) {
 	var (
 		account *gtsmodel.Account
 		uriStr  = uri.String()
@@ -126,9 +150,8 @@ func (d *Dereferencer) getAccountByURI(ctx context.Context, requestUser string,
 	)
 
 	// Search the database for existing account with URI.
+	// URI is unique so if we get a hit it's that account for sure.
 	account, err = d.state.DB.GetAccountByURI(
-		// request a barebones object, it may be in the
-		// db but with related models not yet dereferenced.
 		gtscontext.SetBarebones(ctx),
 		uriStr,
 	)
@@ -136,13 +159,20 @@ func (d *Dereferencer) getAccountByURI(ctx context.Context, requestUser string,
 		return nil, nil, gtserror.Newf("error checking database for account %s by uri: %w", uriStr, err)
 	}
 
-	if account == nil {
-		// Else, search the database for existing by URL.
-		account, err = d.state.DB.GetAccountByURL(
+	if account == nil && tryURL {
+		// Else if we're permitted, search the database for *ONE*
+		// account with this URL. This can return multiple hits
+		// so check for ErrMultipleEntries. If we get exactly one
+		// hit it's *probably* the account we're looking for.
+		account, err = d.state.DB.GetOneAccountByURL(
 			gtscontext.SetBarebones(ctx),
 			uriStr,
 		)
-		if err != nil && !errors.Is(err, db.ErrNoEntries) {
+		if err != nil && !errorsv2.IsV2(
+			err,
+			db.ErrNoEntries,
+			db.ErrMultipleEntries,
+		) {
 			return nil, nil, gtserror.Newf("error checking database for account %s by url: %w", uriStr, err)
 		}
 	}
diff --git a/internal/federation/dereferencing/account_test.go b/internal/federation/dereferencing/account_test.go
index 8bcfce2d2..4af885468 100644
--- a/internal/federation/dereferencing/account_test.go
+++ b/internal/federation/dereferencing/account_test.go
@@ -54,6 +54,7 @@ func (suite *AccountTestSuite) TestDereferenceGroup() {
 		context.Background(),
 		fetchingAccount.Username,
 		groupURL,
+		false,
 	)
 	suite.NoError(err)
 	suite.NotNil(group)
@@ -67,7 +68,7 @@ func (suite *AccountTestSuite) TestDereferenceGroup() {
 	dbGroup, err := suite.db.GetAccountByURI(context.Background(), group.URI)
 	suite.NoError(err)
 	suite.Equal(group.ID, dbGroup.ID)
-	suite.Equal(ap.ActorGroup, dbGroup.ActorType)
+	suite.Equal(ap.ActorGroup, dbGroup.ActorType.String())
 }
 
 func (suite *AccountTestSuite) TestDereferenceService() {
@@ -78,6 +79,7 @@ func (suite *AccountTestSuite) TestDereferenceService() {
 		context.Background(),
 		fetchingAccount.Username,
 		serviceURL,
+		false,
 	)
 	suite.NoError(err)
 	suite.NotNil(service)
@@ -91,7 +93,7 @@ func (suite *AccountTestSuite) TestDereferenceService() {
 	dbService, err := suite.db.GetAccountByURI(context.Background(), service.URI)
 	suite.NoError(err)
 	suite.Equal(service.ID, dbService.ID)
-	suite.Equal(ap.ActorService, dbService.ActorType)
+	suite.Equal(ap.ActorService, dbService.ActorType.String())
 	suite.Equal("example.org", dbService.Domain)
 }
 
@@ -110,6 +112,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsRemoteURL() {
 		context.Background(),
 		fetchingAccount.Username,
 		testrig.URLMustParse(targetAccount.URI),
+		false,
 	)
 	suite.NoError(err)
 	suite.NotNil(fetchedAccount)
@@ -129,6 +132,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsRemoteURLNoSharedInb
 		context.Background(),
 		fetchingAccount.Username,
 		testrig.URLMustParse(targetAccount.URI),
+		false,
 	)
 	suite.NoError(err)
 	suite.NotNil(fetchedAccount)
@@ -143,6 +147,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsUsername() {
 		context.Background(),
 		fetchingAccount.Username,
 		testrig.URLMustParse(targetAccount.URI),
+		false,
 	)
 	suite.NoError(err)
 	suite.NotNil(fetchedAccount)
@@ -157,6 +162,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsUsernameDomain() {
 		context.Background(),
 		fetchingAccount.Username,
 		testrig.URLMustParse(targetAccount.URI),
+		false,
 	)
 	suite.NoError(err)
 	suite.NotNil(fetchedAccount)
@@ -213,6 +219,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountWithUnknownUserURI() {
 		context.Background(),
 		fetchingAccount.Username,
 		testrig.URLMustParse("http://localhost:8080/users/thisaccountdoesnotexist"),
+		false,
 	)
 	suite.True(gtserror.IsUnretrievable(err))
 	suite.EqualError(err, db.ErrNoEntries.Error())
@@ -265,7 +272,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountByRedirect() {
 	uri := testrig.URLMustParse("https://this-will-be-redirected.butts/")
 
 	// Try dereference the test URI, since it correctly redirects to us it should return our account.
-	account, accountable, err := suite.dereferencer.GetAccountByURI(ctx, fetchingAccount.Username, uri)
+	account, accountable, err := suite.dereferencer.GetAccountByURI(ctx, fetchingAccount.Username, uri, false)
 	suite.NoError(err)
 	suite.Nil(accountable)
 	suite.NotNil(account)
@@ -318,7 +325,7 @@ func (suite *AccountTestSuite) TestDereferenceMasqueradingLocalAccount() {
 	)
 
 	// Try dereference the test URI, since it correctly redirects to us it should return our account.
-	account, accountable, err := suite.dereferencer.GetAccountByURI(ctx, fetchingAccount.Username, uri)
+	account, accountable, err := suite.dereferencer.GetAccountByURI(ctx, fetchingAccount.Username, uri, false)
 	suite.NotNil(err)
 	suite.Nil(account)
 	suite.Nil(accountable)
@@ -341,6 +348,7 @@ func (suite *AccountTestSuite) TestDereferenceRemoteAccountWithNonMatchingURI()
 		context.Background(),
 		fetchingAccount.Username,
 		testrig.URLMustParse(remoteAltURI),
+		false,
 	)
 	suite.Equal(err.Error(), fmt.Sprintf("enrichAccount: account uri %s does not match %s", remoteURI, remoteAltURI))
 	suite.Nil(fetchedAccount)
@@ -357,6 +365,7 @@ func (suite *AccountTestSuite) TestDereferenceRemoteAccountWithUnexpectedKeyChan
 	remoteAcc, _, err := suite.dereferencer.GetAccountByURI(ctx,
 		fetchingAcc.Username,
 		testrig.URLMustParse(remoteURI),
+		false,
 	)
 	suite.NoError(err)
 	suite.NotNil(remoteAcc)
@@ -395,6 +404,7 @@ func (suite *AccountTestSuite) TestDereferenceRemoteAccountWithExpectedKeyChange
 	remoteAcc, _, err := suite.dereferencer.GetAccountByURI(ctx,
 		fetchingAcc.Username,
 		testrig.URLMustParse(remoteURI),
+		false,
 	)
 	suite.NoError(err)
 	suite.NotNil(remoteAcc)
@@ -436,6 +446,7 @@ func (suite *AccountTestSuite) TestRefreshFederatedRemoteAccountWithKeyChange()
 	remoteAcc, _, err := suite.dereferencer.GetAccountByURI(ctx,
 		fetchingAcc.Username,
 		testrig.URLMustParse(remoteURI),
+		false,
 	)
 	suite.NoError(err)
 	suite.NotNil(remoteAcc)
diff --git a/internal/federation/dereferencing/status.go b/internal/federation/dereferencing/status.go
index 2718187cf..d99bae15b 100644
--- a/internal/federation/dereferencing/status.go
+++ b/internal/federation/dereferencing/status.go
@@ -454,7 +454,8 @@ func (d *Dereferencer) enrichStatus(
 
 	// Ensure we have the author account of the status dereferenced (+ up-to-date). If this is a new status
 	// (i.e. status.AccountID == "") then any error here is irrecoverable. status.AccountID must ALWAYS be set.
-	if _, _, err := d.getAccountByURI(ctx, requestUser, attributedTo); err != nil && status.AccountID == "" {
+	// We want the exact URI match here as well, not the imprecise URL match.
+	if _, _, err := d.getAccountByURI(ctx, requestUser, attributedTo, false); err != nil && status.AccountID == "" {
 
 		// Note that we specifically DO NOT wrap the error, instead collapsing it as string.
 		// Errors fetching an account do not necessarily relate to dereferencing the status.
@@ -671,7 +672,7 @@ func (d *Dereferencer) fetchStatusMentions(
 
 		// Search existing status for a mention already stored,
 		// else ensure new mention's target account is populated.
-		mention, alreadyExists, err = d.getPopulatedMention(ctx,
+		mention, alreadyExists, err = d.populateMentionTarget(ctx,
 			requestUser,
 			existing,
 			mention,
@@ -1290,7 +1291,7 @@ func (d *Dereferencer) handleStatusEdit(
 	return cols, nil
 }
 
-// getPopulatedMention tries to populate the given
+// populateMentionTarget tries to populate the given
 // mention with the correct TargetAccount and (if not
 // yet set) TargetAccountURI, returning the populated
 // mention.
@@ -1302,7 +1303,13 @@ func (d *Dereferencer) handleStatusEdit(
 // Otherwise, this function will try to parse first
 // the Href of the mention, and then the namestring,
 // to see who it targets, and go fetch that account.
-func (d *Dereferencer) getPopulatedMention(
+//
+// Note: Ordinarily it would make sense to try the
+// namestring first, as it definitely can't be a URL
+// rather than a URI, but because some remotes do
+// silly things like only provide `@username` instead
+// of `@username@domain`, we try by URI first.
+func (d *Dereferencer) populateMentionTarget(
 	ctx context.Context,
 	requestUser string,
 	existing *gtsmodel.Status,
@@ -1312,8 +1319,9 @@ func (d *Dereferencer) getPopulatedMention(
 	bool, // True if mention already exists in the DB.
 	error,
 ) {
-	// Mentions can be created using Name or Href.
-	// Prefer Href (TargetAccountURI), fall back to Name.
+	// Mentions can be created using `name` or `href`.
+	//
+	// Prefer `href` (TargetAccountURI), fall back to Name.
 	if mention.TargetAccountURI != "" {
 
 		// Look for existing mention with target account's URI, if so use this.
@@ -1323,19 +1331,24 @@ func (d *Dereferencer) getPopulatedMention(
 		}
 
 		// Ensure that mention account URI is parseable.
-		accountURI, err := url.Parse(mention.TargetAccountURI)
+		targetAccountURI, err := url.Parse(mention.TargetAccountURI)
 		if err != nil {
 			err := gtserror.Newf("invalid account uri %q: %w", mention.TargetAccountURI, err)
 			return nil, false, err
 		}
 
-		// Ensure we have  account of the mention target dereferenced.
+		// Ensure we have the account of
+		// the mention target dereferenced.
+		//
+		// Use exact URI match only, not URL,
+		// as we want to be precise here.
 		mention.TargetAccount, _, err = d.getAccountByURI(ctx,
 			requestUser,
-			accountURI,
+			targetAccountURI,
+			false,
 		)
 		if err != nil {
-			err := gtserror.Newf("failed to dereference account %s: %w", accountURI, err)
+			err := gtserror.Newf("failed to dereference account %s: %w", targetAccountURI, err)
 			return nil, false, err
 		}
 	} else {
@@ -1353,17 +1366,32 @@ func (d *Dereferencer) getPopulatedMention(
 			return existingMention, true, nil
 		}
 
-		// Ensure we have the account of the mention target dereferenced.
+		// Ensure we have the account of
+		// the mention target dereferenced.
+		//
+		// This might fail if the remote does
+		// something silly like only setting
+		// `@username` and not `@username@domain`.
 		mention.TargetAccount, _, err = d.getAccountByUsernameDomain(ctx,
 			requestUser,
 			username,
 			domain,
 		)
-		if err != nil {
+		if err != nil && !errors.Is(err, db.ErrNoEntries) {
 			err := gtserror.Newf("failed to dereference account %s: %w", mention.NameString, err)
 			return nil, false, err
 		}
 
+		if mention.TargetAccount == nil {
+			// Probably failed for abovementioned
+			// silly reason. Nothing we can do about it.
+			err := gtserror.Newf(
+				"failed to populate mention target account (badly formatted namestring?) %s: %w",
+				mention.NameString, err,
+			)
+			return nil, false, err
+		}
+
 		// Look for existing mention with target account's URI, if so use this.
 		existingMention, ok = existing.GetMentionByTargetURI(mention.TargetAccountURI)
 		if ok && existingMention.ID != "" {