From 97f54533781139407f7d33aa0bdb80d0bf3264f6 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Tue, 29 Nov 2022 10:24:55 +0100 Subject: [PATCH] [chore] Tidy up some of the search logic (#1082) * start refactoring some of the search + deref logic * add tests for search api * rename GetRemoteAccount + GetRemoteStatus * make search function a bit simpler + clearer * fix little fucky wucky uwu owo i'm just a little guy * update faulty switch statements * update test to use storage struct * redo switches for clarity * reduce repeated logic in search tests * fastfail getstatus by uri * debug log + trace log better * add implementation note * return early if no result for namestring search * return + check on dereferencing error types * errors hah what errors * remove unneeded error type alias, add custom error text during stringification itself * fix a woops recursion :see_no_evil: Signed-off-by: kim Co-authored-by: kim --- internal/api/client/search/search_test.go | 115 ++++++++ internal/api/client/search/searchget_test.go | 240 +++++++++++++++ internal/federation/dereference.go | 8 +- internal/federation/dereferencing/account.go | 276 +++++++++++------- .../federation/dereferencing/account_test.go | 40 +-- internal/federation/dereferencing/announce.go | 2 +- .../federation/dereferencing/dereferencer.go | 10 +- internal/federation/dereferencing/error.go | 132 +++++++++ internal/federation/dereferencing/status.go | 109 ++++--- .../federation/dereferencing/status_test.go | 6 +- internal/federation/dereferencing/thread.go | 4 +- internal/federation/federatingprotocol.go | 2 +- internal/federation/federator.go | 10 +- internal/processing/account/get.go | 2 +- .../processing/federation/getfollowers.go | 2 +- .../processing/federation/getfollowing.go | 2 +- internal/processing/federation/getoutbox.go | 2 +- internal/processing/federation/getstatus.go | 2 +- .../processing/federation/getstatusreplies.go | 2 +- internal/processing/federation/getuser.go | 2 +- internal/processing/fromfederator.go | 12 +- internal/processing/search.go | 190 ++++++------ internal/processing/util.go | 2 +- 23 files changed, 890 insertions(+), 282 deletions(-) create mode 100644 internal/api/client/search/search_test.go create mode 100644 internal/api/client/search/searchget_test.go create mode 100644 internal/federation/dereferencing/error.go diff --git a/internal/api/client/search/search_test.go b/internal/api/client/search/search_test.go new file mode 100644 index 000000000..11b5b80b2 --- /dev/null +++ b/internal/api/client/search/search_test.go @@ -0,0 +1,115 @@ +/* + GoToSocial + Copyright (C) 2021-2022 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 search_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/api/client/search" + "github.com/superseriousbusiness/gotosocial/internal/concurrency" + "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/email" + "github.com/superseriousbusiness/gotosocial/internal/federation" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/media" + "github.com/superseriousbusiness/gotosocial/internal/messages" + "github.com/superseriousbusiness/gotosocial/internal/oauth" + "github.com/superseriousbusiness/gotosocial/internal/processing" + "github.com/superseriousbusiness/gotosocial/internal/storage" + "github.com/superseriousbusiness/gotosocial/testrig" +) + +type SearchStandardTestSuite struct { + // standard suite interfaces + suite.Suite + db db.DB + storage *storage.Driver + mediaManager media.Manager + federator federation.Federator + processor processing.Processor + emailSender email.Sender + sentEmails map[string]string + + // 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 + + // module being tested + searchModule *search.Module +} + +func (suite *SearchStandardTestSuite) SetupSuite() { + suite.testTokens = testrig.NewTestTokens() + suite.testClients = testrig.NewTestClients() + suite.testApplications = testrig.NewTestApplications() + suite.testUsers = testrig.NewTestUsers() + suite.testAccounts = testrig.NewTestAccounts() +} + +func (suite *SearchStandardTestSuite) SetupTest() { + testrig.InitTestConfig() + testrig.InitTestLog() + + fedWorker := concurrency.NewWorkerPool[messages.FromFederator](-1, -1) + clientWorker := concurrency.NewWorkerPool[messages.FromClientAPI](-1, -1) + + suite.db = testrig.NewTestDB() + suite.storage = testrig.NewInMemoryStorage() + suite.mediaManager = testrig.NewTestMediaManager(suite.db, suite.storage) + suite.federator = testrig.NewTestFederator(suite.db, testrig.NewTestTransportController(testrig.NewMockHTTPClient(nil, "../../../../testrig/media"), suite.db, fedWorker), suite.storage, suite.mediaManager, fedWorker) + suite.sentEmails = make(map[string]string) + suite.emailSender = testrig.NewEmailSender("../../../../web/template/", suite.sentEmails) + suite.processor = testrig.NewTestProcessor(suite.db, suite.storage, suite.federator, suite.emailSender, suite.mediaManager, clientWorker, fedWorker) + suite.searchModule = search.New(suite.processor).(*search.Module) + testrig.StandardDBSetup(suite.db, nil) + testrig.StandardStorageSetup(suite.storage, "../../../../testrig/media") + + suite.NoError(suite.processor.Start()) +} + +func (suite *SearchStandardTestSuite) TearDownTest() { + testrig.StandardDBTeardown(suite.db) + testrig.StandardStorageTeardown(suite.storage) +} + +func (suite *SearchStandardTestSuite) newContext(recorder *httptest.ResponseRecorder, requestPath string) *gin.Context { + ctx, _ := testrig.CreateGinTestContext(recorder, nil) + ctx.Set(oauth.SessionAuthorizedAccount, suite.testAccounts["local_account_1"]) + ctx.Set(oauth.SessionAuthorizedToken, oauth.DBTokenToToken(suite.testTokens["local_account_1"])) + ctx.Set(oauth.SessionAuthorizedApplication, suite.testApplications["application_1"]) + ctx.Set(oauth.SessionAuthorizedUser, suite.testUsers["local_account_1"]) + + protocol := config.GetProtocol() + host := config.GetHost() + + baseURI := fmt.Sprintf("%s://%s", protocol, host) + requestURI := fmt.Sprintf("%s/%s", baseURI, requestPath) + + ctx.Request = httptest.NewRequest(http.MethodGet, requestURI, nil) // the endpoint we're hitting + ctx.Request.Header.Set("accept", "application/json") + return ctx +} diff --git a/internal/api/client/search/searchget_test.go b/internal/api/client/search/searchget_test.go new file mode 100644 index 000000000..bcbe83baf --- /dev/null +++ b/internal/api/client/search/searchget_test.go @@ -0,0 +1,240 @@ +/* + GoToSocial + Copyright (C) 2021-2022 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 search_test + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/api/client/search" + apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" +) + +type SearchGetTestSuite struct { + SearchStandardTestSuite +} + +func (suite *SearchGetTestSuite) testSearch(query string, resolve bool, expectedHTTPStatus int) (*apimodel.SearchResult, error) { + requestPath := fmt.Sprintf("%s?q=%s&resolve=%t", search.BasePathV1, query, resolve) + recorder := httptest.NewRecorder() + + ctx := suite.newContext(recorder, requestPath) + + suite.searchModule.SearchGETHandler(ctx) + + result := recorder.Result() + defer result.Body.Close() + + if resultCode := recorder.Code; expectedHTTPStatus != resultCode { + return nil, fmt.Errorf("expected %d got %d", expectedHTTPStatus, resultCode) + } + + b, err := ioutil.ReadAll(result.Body) + if err != nil { + return nil, err + } + + searchResult := &apimodel.SearchResult{} + if err := json.Unmarshal(b, searchResult); err != nil { + return nil, err + } + + return searchResult, nil +} + +func (suite *SearchGetTestSuite) TestSearchRemoteAccountByURI() { + query := "https://unknown-instance.com/users/brand_new_person" + resolve := true + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + if !suite.Len(searchResult.Accounts, 1) { + suite.FailNow("expected 1 account in search results but got 0") + } + + gotAccount := searchResult.Accounts[0] + suite.NotNil(gotAccount) +} + +func (suite *SearchGetTestSuite) TestSearchRemoteAccountByNamestring() { + query := "@brand_new_person@unknown-instance.com" + resolve := true + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + if !suite.Len(searchResult.Accounts, 1) { + suite.FailNow("expected 1 account in search results but got 0") + } + + gotAccount := searchResult.Accounts[0] + suite.NotNil(gotAccount) +} + +func (suite *SearchGetTestSuite) TestSearchRemoteAccountByNamestringNoLeadingAt() { + query := "brand_new_person@unknown-instance.com" + resolve := true + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + if !suite.Len(searchResult.Accounts, 1) { + suite.FailNow("expected 1 account in search results but got 0") + } + + gotAccount := searchResult.Accounts[0] + suite.NotNil(gotAccount) +} + +func (suite *SearchGetTestSuite) TestSearchRemoteAccountByNamestringNoResolve() { + query := "@brand_new_person@unknown-instance.com" + resolve := false + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Len(searchResult.Accounts, 0) +} + +func (suite *SearchGetTestSuite) TestSearchLocalAccountByNamestring() { + query := "@the_mighty_zork" + resolve := false + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + if !suite.Len(searchResult.Accounts, 1) { + suite.FailNow("expected 1 account in search results but got 0") + } + + gotAccount := searchResult.Accounts[0] + suite.NotNil(gotAccount) +} + +func (suite *SearchGetTestSuite) TestSearchLocalAccountByNamestringWithDomain() { + query := "@the_mighty_zork@localhost:8080" + resolve := false + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + if !suite.Len(searchResult.Accounts, 1) { + suite.FailNow("expected 1 account in search results but got 0") + } + + gotAccount := searchResult.Accounts[0] + suite.NotNil(gotAccount) +} + +func (suite *SearchGetTestSuite) TestSearchNonexistingLocalAccountByNamestringResolveTrue() { + query := "@somone_made_up@localhost:8080" + resolve := true + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Len(searchResult.Accounts, 0) +} + +func (suite *SearchGetTestSuite) TestSearchLocalAccountByURI() { + query := "http://localhost:8080/users/the_mighty_zork" + resolve := false + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + if !suite.Len(searchResult.Accounts, 1) { + suite.FailNow("expected 1 account in search results but got 0") + } + + gotAccount := searchResult.Accounts[0] + suite.NotNil(gotAccount) +} + +func (suite *SearchGetTestSuite) TestSearchLocalAccountByURL() { + query := "http://localhost:8080/@the_mighty_zork" + resolve := false + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + if !suite.Len(searchResult.Accounts, 1) { + suite.FailNow("expected 1 account in search results but got 0") + } + + gotAccount := searchResult.Accounts[0] + suite.NotNil(gotAccount) +} + +func (suite *SearchGetTestSuite) TestSearchNonexistingLocalAccountByURL() { + query := "http://localhost:8080/@the_shmighty_shmork" + resolve := true + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Len(searchResult.Accounts, 0) +} + +func (suite *SearchGetTestSuite) TestSearchStatusByURL() { + query := "https://turnip.farm/users/turniplover6969/statuses/70c53e54-3146-42d5-a630-83c8b6c7c042" + resolve := true + + searchResult, err := suite.testSearch(query, resolve, http.StatusOK) + if err != nil { + suite.FailNow(err.Error()) + } + + if !suite.Len(searchResult.Statuses, 1) { + suite.FailNow("expected 1 status in search results but got 0") + } + + gotStatus := searchResult.Statuses[0] + suite.NotNil(gotStatus) +} + +func TestSearchGetTestSuite(t *testing.T) { + suite.Run(t, &SearchGetTestSuite{}) +} diff --git a/internal/federation/dereference.go b/internal/federation/dereference.go index 6095a4d52..dcb4cc631 100644 --- a/internal/federation/dereference.go +++ b/internal/federation/dereference.go @@ -27,12 +27,12 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) -func (f *federator) GetRemoteAccount(ctx context.Context, params dereferencing.GetRemoteAccountParams) (*gtsmodel.Account, error) { - return f.dereferencer.GetRemoteAccount(ctx, params) +func (f *federator) GetAccount(ctx context.Context, params dereferencing.GetAccountParams) (*gtsmodel.Account, error) { + return f.dereferencer.GetAccount(ctx, params) } -func (f *federator) GetRemoteStatus(ctx context.Context, username string, remoteStatusID *url.URL, refetch, includeParent bool) (*gtsmodel.Status, ap.Statusable, error) { - return f.dereferencer.GetRemoteStatus(ctx, username, remoteStatusID, refetch, includeParent) +func (f *federator) GetStatus(ctx context.Context, username string, remoteStatusID *url.URL, refetch, includeParent bool) (*gtsmodel.Status, ap.Statusable, error) { + return f.dereferencer.GetStatus(ctx, username, remoteStatusID, refetch, includeParent) } func (f *federator) EnrichRemoteStatus(ctx context.Context, username string, status *gtsmodel.Status, includeParent bool) (*gtsmodel.Status, error) { diff --git a/internal/federation/dereferencing/account.go b/internal/federation/dereferencing/account.go index e351cd2fd..48ee9f1d7 100644 --- a/internal/federation/dereferencing/account.go +++ b/internal/federation/dereferencing/account.go @@ -50,20 +50,20 @@ func instanceAccount(account *gtsmodel.Account) bool { (account.Username == "internal.fetch" && strings.Contains(account.Note, "internal service actor")) } -// GetRemoteAccountParams wraps parameters for a remote account lookup. -type GetRemoteAccountParams struct { +// GetAccountParams wraps parameters for an account lookup. +type GetAccountParams struct { // The username of the user doing the lookup request (optional). // If not set, then the GtS instance account will be used to do the lookup. RequestingUsername string - // The ActivityPub URI of the remote account (optional). - // If not set (nil), the ActivityPub URI of the remote account will be discovered + // The ActivityPub URI of the account (optional). + // If not set (nil), the ActivityPub URI of the account will be discovered // via webfinger, so you must set RemoteAccountUsername and RemoteAccountHost // if this parameter is not set. RemoteAccountID *url.URL - // The username of the remote account (optional). + // The username of the account (optional). // If RemoteAccountID is not set, then this value must be set. RemoteAccountUsername string - // The host of the remote account (optional). + // The host of the account (optional). // If RemoteAccountID is not set, then this value must be set. RemoteAccountHost string // Whether to do a blocking call to the remote instance. If true, @@ -82,17 +82,51 @@ type GetRemoteAccountParams struct { PartialAccount *gtsmodel.Account } -// GetRemoteAccount completely dereferences a remote account, converts it to a GtS model account, +type lookupType int + +const ( + lookupPartialLocal lookupType = iota + lookupPartial + lookupURILocal + lookupURI + lookupMentionLocal + lookupMention + lookupBad +) + +func getLookupType(params GetAccountParams) lookupType { + switch { + case params.PartialAccount != nil: + if params.PartialAccount.Domain == "" || params.PartialAccount.Domain == config.GetHost() || params.PartialAccount.Domain == config.GetAccountDomain() { + return lookupPartialLocal + } + return lookupPartial + case params.RemoteAccountID != nil: + if host := params.RemoteAccountID.Host; host == config.GetHost() || host == config.GetAccountDomain() { + return lookupURILocal + } + return lookupURI + case params.RemoteAccountUsername != "": + if params.RemoteAccountHost == "" || params.RemoteAccountHost == config.GetHost() || params.RemoteAccountHost == config.GetAccountDomain() { + return lookupMentionLocal + } + return lookupMention + default: + return lookupBad + } +} + +// GetAccount completely dereferences an account, converts it to a GtS model account, // puts or updates it in the database (if necessary), and returns it to a caller. // -// If a local account is passed into this function for whatever reason (hey, it happens!), then it -// will be returned from the database without making any remote calls. +// GetAccount will guard against trying to do http calls to fetch an account that belongs to this instance. +// Instead of making calls, it will just return the account early if it finds it, or return an error. // // Even if a fastfail context is used, and something goes wrong, an account might still be returned instead // of an error, if we already had the account in our database (in other words, if we just needed to try // fingering/refreshing the account again). The rationale for this is that it's more useful to be able // to provide *something* to the caller, even if that something is not necessarily 100% up to date. -func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountParams) (foundAccount *gtsmodel.Account, err error) { +func (d *deref) GetAccount(ctx context.Context, params GetAccountParams) (foundAccount *gtsmodel.Account, err error) { /* In this function we want to retrieve a gtsmodel representation of a remote account, with its proper accountDomain set, while making as few calls to remote instances as possible to save time and bandwidth. @@ -113,88 +147,93 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar from that. */ - skipResolve := params.SkipResolve - // this first step checks if we have the // account in the database somewhere already, // or if we've been provided it as a partial - switch { - case params.PartialAccount != nil: + switch getLookupType(params) { + case lookupPartialLocal: + params.SkipResolve = true + fallthrough + case lookupPartial: foundAccount = params.PartialAccount - if foundAccount.Domain == "" || foundAccount.Domain == config.GetHost() || foundAccount.Domain == config.GetAccountDomain() { - // this is actually a local account, - // make sure we don't try to resolve - skipResolve = true - } - case params.RemoteAccountID != nil: - uri := params.RemoteAccountID - host := uri.Host - if host == config.GetHost() || host == config.GetAccountDomain() { - // this is actually a local account, - // make sure we don't try to resolve - skipResolve = true + case lookupURILocal: + params.SkipResolve = true + fallthrough + case lookupURI: + // see if we have this in the db already with this uri/url + uri := params.RemoteAccountID.String() + + if a, dbErr := d.db.GetAccountByURI(ctx, uri); dbErr == nil { + // got it, break here to leave early + foundAccount = a + break + } else if !errors.Is(dbErr, db.ErrNoEntries) { + // a real error + err = newErrDB(fmt.Errorf("GetRemoteAccount: unexpected error while looking for account with uri %s: %w", uri, dbErr)) + break } - if a, dbErr := d.db.GetAccountByURI(ctx, uri.String()); dbErr == nil { + // dbErr was just db.ErrNoEntries so search by url instead + if a, dbErr := d.db.GetAccountByURL(ctx, uri); dbErr == nil { + // got it foundAccount = a - } else if dbErr != db.ErrNoEntries { - err = fmt.Errorf("GetRemoteAccount: database error looking for account with uri %s: %s", uri, err) + break + } else if !errors.Is(dbErr, db.ErrNoEntries) { + // a real error + err = newErrDB(fmt.Errorf("GetRemoteAccount: unexpected error while looking for account with url %s: %w", uri, dbErr)) + break } - case params.RemoteAccountUsername != "" && (params.RemoteAccountHost == "" || params.RemoteAccountHost == config.GetHost() || params.RemoteAccountHost == config.GetAccountDomain()): - // either no domain is provided or this seems - // to be a local account, so don't resolve - skipResolve = true - - if a, dbErr := d.db.GetAccountByUsernameDomain(ctx, params.RemoteAccountUsername, ""); dbErr == nil { - foundAccount = a - } else if dbErr != db.ErrNoEntries { - err = fmt.Errorf("GetRemoteAccount: database error looking for local account with username %s: %s", params.RemoteAccountUsername, err) - } - case params.RemoteAccountUsername != "" && params.RemoteAccountHost != "": + case lookupMentionLocal: + params.SkipResolve = true + params.RemoteAccountHost = "" + fallthrough + case lookupMention: + // see if we have this in the db already with this username/host if a, dbErr := d.db.GetAccountByUsernameDomain(ctx, params.RemoteAccountUsername, params.RemoteAccountHost); dbErr == nil { foundAccount = a - } else if dbErr != db.ErrNoEntries { - err = fmt.Errorf("GetRemoteAccount: database error looking for account with username %s and domain %s: %s", params.RemoteAccountUsername, params.RemoteAccountHost, err) + } else if !errors.Is(dbErr, db.ErrNoEntries) { + // a real error + err = newErrDB(fmt.Errorf("GetRemoteAccount: unexpected error while looking for account %s: %w", params.RemoteAccountUsername, dbErr)) } default: - err = errors.New("GetRemoteAccount: no identifying parameters were set so we cannot get account") + err = newErrBadRequest(errors.New("GetRemoteAccount: no identifying parameters were set so we cannot get account")) } + // bail if we've set a real error, and not just no entries in the db if err != nil { return } - if skipResolve { - // if we can't resolve, return already - // since there's nothing more we can do + if params.SkipResolve { + // if we can't resolve, return already since there's nothing more we can do if foundAccount == nil { - err = errors.New("GetRemoteAccount: couldn't retrieve account locally and won't try to resolve it") + err = newErrNotRetrievable(errors.New("GetRemoteAccount: couldn't retrieve account locally and not allowed to resolve it")) } return } - var accountable ap.Accountable - if params.RemoteAccountUsername == "" || params.RemoteAccountHost == "" { - // try to populate the missing params - // the first one is easy ... - params.RemoteAccountHost = params.RemoteAccountID.Host - // ... but we still need the username so we can do a finger for the accountDomain + // if we reach this point, we have some remote calls to make - // check if we got the account earlier + var accountable ap.Accountable + if params.RemoteAccountUsername == "" && params.RemoteAccountHost == "" { + // if we're still missing some params, try to populate them now + params.RemoteAccountHost = params.RemoteAccountID.Host if foundAccount != nil { + // username is easy if we found something already params.RemoteAccountUsername = foundAccount.Username } else { - // if we didn't already have it, we have dereference it from remote and just... - accountable, err = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID) - if err != nil { - err = fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err) + // if we didn't already have it, we have to dereference it from remote + var derefErr error + accountable, derefErr = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID) + if derefErr != nil { + err = wrapDerefError(derefErr, "GetRemoteAccount: error dereferencing Accountable") return } - // ... take the username (for now) - params.RemoteAccountUsername, err = ap.ExtractPreferredUsername(accountable) - if err != nil { - err = fmt.Errorf("GetRemoteAccount: error extracting accountable username: %s", err) + var apError error + params.RemoteAccountUsername, apError = ap.ExtractPreferredUsername(accountable) + if apError != nil { + err = newErrOther(fmt.Errorf("GetRemoteAccount: error extracting Accountable username: %w", apError)) return } } @@ -221,11 +260,24 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar // - we were passed a partial account in params OR // - we haven't webfingered the account for two days AND the account isn't an instance account var fingered time.Time - if params.RemoteAccountID == nil || foundAccount == nil || params.PartialAccount != nil || (foundAccount.LastWebfingeredAt.Before(time.Now().Add(webfingerInterval)) && !instanceAccount(foundAccount)) { - accountDomain, params.RemoteAccountID, err = d.fingerRemoteAccount(ctx, params.RequestingUsername, params.RemoteAccountUsername, params.RemoteAccountHost) - if err != nil { - err = fmt.Errorf("GetRemoteAccount: error while fingering: %s", err) - return + var refreshFinger bool + if foundAccount != nil { + refreshFinger = foundAccount.LastWebfingeredAt.Before(time.Now().Add(webfingerInterval)) && !instanceAccount(foundAccount) + } + + if params.RemoteAccountID == nil || foundAccount == nil || params.PartialAccount != nil || refreshFinger { + if ad, accountURI, fingerError := d.fingerRemoteAccount(ctx, params.RequestingUsername, params.RemoteAccountUsername, params.RemoteAccountHost); fingerError != nil { + if !refreshFinger { + // only return with an error if this wasn't just a refresh finger; + // that is, if we actually *needed* to finger in order to get the account, + // otherwise we can just continue and we'll try again in 2 days + err = newErrNotRetrievable(fmt.Errorf("GetRemoteAccount: error while fingering: %w", fingerError)) + return + } + log.Infof("error doing non-vital webfinger refresh call to %s: %s", params.RemoteAccountHost, err) + } else { + accountDomain = ad + params.RemoteAccountID = accountURI } fingered = time.Now() } @@ -234,24 +286,30 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar // if we just fingered and now have a discovered account domain but still no account, // we should do a final lookup in the database with the discovered username + accountDomain // to make absolutely sure we don't already have this account - a := >smodel.Account{} - where := []db.Where{{Key: "username", Value: params.RemoteAccountUsername}, {Key: "domain", Value: accountDomain}} - if dbErr := d.db.GetWhere(ctx, where, a); dbErr == nil { + if a, dbErr := d.db.GetAccountByUsernameDomain(ctx, params.RemoteAccountUsername, accountDomain); dbErr == nil { foundAccount = a - } else if dbErr != db.ErrNoEntries { - err = fmt.Errorf("GetRemoteAccount: database error looking for account with username %s and host %s: %s", params.RemoteAccountUsername, params.RemoteAccountHost, err) + } else if !errors.Is(dbErr, db.ErrNoEntries) { + // a real error + err = newErrDB(fmt.Errorf("GetRemoteAccount: unexpected error while looking for account %s: %w", params.RemoteAccountUsername, dbErr)) return } } - // we may also have some extra information already, like the account we had in the db, or the + // we may have some extra information already, like the account we had in the db, or the // accountable representation that we dereferenced from remote if foundAccount == nil { - // we still don't have the account, so deference it if we didn't earlier + // if we still don't have a remoteAccountID here we're boned + if params.RemoteAccountID == nil { + err = newErrNotRetrievable(errors.New("GetRemoteAccount: could not populate find an account nor populate params.RemoteAccountID")) + return + } + + // deference accountable if we didn't earlier if accountable == nil { - accountable, err = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID) - if err != nil { - err = fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err) + var derefErr error + accountable, derefErr = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID) + if derefErr != nil { + err = wrapDerefError(derefErr, "GetRemoteAccount: error dereferencing Accountable") return } } @@ -259,7 +317,7 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar // then convert foundAccount, err = d.typeConverter.ASRepresentationToAccount(ctx, accountable, accountDomain, false) if err != nil { - err = fmt.Errorf("GetRemoteAccount: error converting accountable to account: %s", err) + err = newErrOther(fmt.Errorf("GetRemoteAccount: error converting Accountable to account: %w", err)) return } @@ -267,23 +325,21 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar var ulid string ulid, err = id.NewRandomULID() if err != nil { - err = fmt.Errorf("GetRemoteAccount: error generating new id for account: %s", err) + err = newErrOther(fmt.Errorf("GetRemoteAccount: error generating new id for account: %w", err)) return } foundAccount.ID = ulid - _, err = d.populateAccountFields(ctx, foundAccount, params.RequestingUsername, params.Blocking) - if err != nil { - err = fmt.Errorf("GetRemoteAccount: error populating further account fields: %s", err) - return + if _, populateErr := d.populateAccountFields(ctx, foundAccount, params.RequestingUsername, params.Blocking); populateErr != nil { + // it's not the end of the world if we can't populate account fields, but we do want to log it + log.Errorf("GetRemoteAccount: error populating further account fields: %s", populateErr) } foundAccount.LastWebfingeredAt = fingered foundAccount.UpdatedAt = time.Now() - err = d.db.PutAccount(ctx, foundAccount) - if err != nil { - err = fmt.Errorf("GetRemoteAccount: error putting new account: %s", err) + if dbErr := d.db.PutAccount(ctx, foundAccount); dbErr != nil { + err = newErrDB(fmt.Errorf("GetRemoteAccount: error putting new account: %w", dbErr)) return } @@ -303,9 +359,10 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar if foundAccount.SharedInboxURI == nil { // we need the accountable for this, so get it if we don't have it yet if accountable == nil { - accountable, err = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID) - if err != nil { - err = fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err) + var derefErr error + accountable, derefErr = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID) + if derefErr != nil { + err = wrapDerefError(derefErr, "GetRemoteAccount: error dereferencing Accountable") return } } @@ -330,10 +387,10 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar // make sure the account fields are populated before returning: // the caller might want to block until everything is loaded - var fieldsChanged bool - fieldsChanged, err = d.populateAccountFields(ctx, foundAccount, params.RequestingUsername, params.Blocking) - if err != nil { - return nil, fmt.Errorf("GetRemoteAccount: error populating remoteAccount fields: %s", err) + fieldsChanged, populateErr := d.populateAccountFields(ctx, foundAccount, params.RequestingUsername, params.Blocking) + if populateErr != nil { + // it's not the end of the world if we can't populate account fields, but we do want to log it + log.Errorf("GetRemoteAccount: error populating further account fields: %s", populateErr) } var fingeredChanged bool @@ -343,9 +400,9 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar } if accountDomainChanged || sharedInboxChanged || fieldsChanged || fingeredChanged { - err = d.db.UpdateAccount(ctx, foundAccount) - if err != nil { - return nil, fmt.Errorf("GetRemoteAccount: error updating remoteAccount: %s", err) + if dbErr := d.db.UpdateAccount(ctx, foundAccount); dbErr != nil { + err = newErrDB(fmt.Errorf("GetRemoteAccount: error updating remoteAccount: %w", err)) + return } } @@ -366,22 +423,22 @@ func (d *deref) dereferenceAccountable(ctx context.Context, username string, rem transport, err := d.transportController.NewTransportForUsername(ctx, username) if err != nil { - return nil, fmt.Errorf("DereferenceAccountable: transport err: %s", err) + return nil, fmt.Errorf("DereferenceAccountable: transport err: %w", err) } b, err := transport.Dereference(ctx, remoteAccountID) if err != nil { - return nil, fmt.Errorf("DereferenceAccountable: error deferencing %s: %s", remoteAccountID.String(), err) + return nil, fmt.Errorf("DereferenceAccountable: error deferencing %s: %w", remoteAccountID.String(), err) } m := make(map[string]interface{}) if err := json.Unmarshal(b, &m); err != nil { - return nil, fmt.Errorf("DereferenceAccountable: error unmarshalling bytes into json: %s", err) + return nil, fmt.Errorf("DereferenceAccountable: error unmarshalling bytes into json: %w", err) } t, err := streams.ToType(ctx, m) if err != nil { - return nil, fmt.Errorf("DereferenceAccountable: error resolving json into ap vocab type: %s", err) + return nil, fmt.Errorf("DereferenceAccountable: error resolving json into ap vocab type: %w", err) } switch t.GetTypeName() { @@ -417,11 +474,11 @@ func (d *deref) dereferenceAccountable(ctx context.Context, username string, rem return p, nil } - return nil, fmt.Errorf("DereferenceAccountable: type name %s not supported", t.GetTypeName()) + return nil, newErrWrongType(fmt.Errorf("DereferenceAccountable: type name %s not supported as Accountable", t.GetTypeName())) } -// populateAccountFields populates any fields on the given account that weren't populated by the initial -// dereferencing. This includes things like header and avatar etc. +// populateAccountFields makes a best effort to populate fields on an account such as emojis, avatar, header. +// Will return true if one of these things changed on the passed-in account. func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Account, requestingUsername string, blocking bool) (bool, error) { // if we're dealing with an instance account, just bail, we don't need to do anything if instanceAccount(account) { @@ -430,10 +487,15 @@ func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Acc accountURI, err := url.Parse(account.URI) if err != nil { - return false, fmt.Errorf("populateAccountFields: couldn't parse account URI %s: %s", account.URI, err) + return false, fmt.Errorf("populateAccountFields: couldn't parse account URI %s: %w", account.URI, err) } - if blocked, err := d.db.IsDomainBlocked(ctx, accountURI.Host); blocked || err != nil { + blocked, dbErr := d.db.IsDomainBlocked(ctx, accountURI.Host) + if dbErr != nil { + return false, fmt.Errorf("populateAccountFields: eror checking for block of domain %s: %w", accountURI.Host, err) + } + + if blocked { return false, fmt.Errorf("populateAccountFields: domain %s is blocked", accountURI.Host) } @@ -441,14 +503,14 @@ func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Acc // fetch the header and avatar if mediaChanged, err := d.fetchRemoteAccountMedia(ctx, account, requestingUsername, blocking); err != nil { - return false, fmt.Errorf("populateAccountFields: error fetching header/avi for account: %s", err) + return false, fmt.Errorf("populateAccountFields: error fetching header/avi for account: %w", err) } else if mediaChanged { changed = mediaChanged } // fetch any emojis used in note, fields, display name, etc if emojisChanged, err := d.fetchRemoteAccountEmojis(ctx, account, requestingUsername); err != nil { - return false, fmt.Errorf("populateAccountFields: error fetching emojis for account: %s", err) + return false, fmt.Errorf("populateAccountFields: error fetching emojis for account: %w", err) } else if emojisChanged { changed = emojisChanged } diff --git a/internal/federation/dereferencing/account_test.go b/internal/federation/dereferencing/account_test.go index 38dc615d5..c5541209f 100644 --- a/internal/federation/dereferencing/account_test.go +++ b/internal/federation/dereferencing/account_test.go @@ -39,7 +39,7 @@ func (suite *AccountTestSuite) TestDereferenceGroup() { fetchingAccount := suite.testAccounts["local_account_1"] groupURL := testrig.URLMustParse("https://unknown-instance.com/groups/some_group") - group, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + group, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountID: groupURL, }) @@ -62,7 +62,7 @@ func (suite *AccountTestSuite) TestDereferenceService() { fetchingAccount := suite.testAccounts["local_account_1"] serviceURL := testrig.URLMustParse("https://owncast.example.org/federation/user/rgh") - service, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + service, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountID: serviceURL, }) @@ -93,7 +93,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsRemoteURL() { fetchingAccount := suite.testAccounts["local_account_1"] targetAccount := suite.testAccounts["local_account_2"] - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountID: testrig.URLMustParse(targetAccount.URI), }) @@ -111,7 +111,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsRemoteURLNoSharedInb suite.FailNow(err.Error()) } - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountID: testrig.URLMustParse(targetAccount.URI), }) @@ -124,7 +124,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsUsername() { fetchingAccount := suite.testAccounts["local_account_1"] targetAccount := suite.testAccounts["local_account_2"] - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountUsername: targetAccount.Username, }) @@ -137,7 +137,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsUsernameDomain() { fetchingAccount := suite.testAccounts["local_account_1"] targetAccount := suite.testAccounts["local_account_2"] - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountUsername: targetAccount.Username, RemoteAccountHost: config.GetHost(), @@ -151,7 +151,7 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsUsernameDomainAndURL fetchingAccount := suite.testAccounts["local_account_1"] targetAccount := suite.testAccounts["local_account_2"] - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountID: testrig.URLMustParse(targetAccount.URI), RemoteAccountUsername: targetAccount.Username, @@ -165,34 +165,40 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountAsUsernameDomainAndURL func (suite *AccountTestSuite) TestDereferenceLocalAccountWithUnknownUsername() { fetchingAccount := suite.testAccounts["local_account_1"] - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountUsername: "thisaccountdoesnotexist", }) - suite.EqualError(err, "GetRemoteAccount: couldn't retrieve account locally and won't try to resolve it") + var errNotRetrievable *dereferencing.ErrNotRetrievable + suite.ErrorAs(err, &errNotRetrievable) + suite.EqualError(err, "item could not be retrieved: GetRemoteAccount: couldn't retrieve account locally and not allowed to resolve it") suite.Nil(fetchedAccount) } func (suite *AccountTestSuite) TestDereferenceLocalAccountWithUnknownUsernameDomain() { fetchingAccount := suite.testAccounts["local_account_1"] - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountUsername: "thisaccountdoesnotexist", RemoteAccountHost: "localhost:8080", }) - suite.EqualError(err, "GetRemoteAccount: couldn't retrieve account locally and won't try to resolve it") + var errNotRetrievable *dereferencing.ErrNotRetrievable + suite.ErrorAs(err, &errNotRetrievable) + suite.EqualError(err, "item could not be retrieved: GetRemoteAccount: couldn't retrieve account locally and not allowed to resolve it") suite.Nil(fetchedAccount) } func (suite *AccountTestSuite) TestDereferenceLocalAccountWithUnknownUserURI() { fetchingAccount := suite.testAccounts["local_account_1"] - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountID: testrig.URLMustParse("http://localhost:8080/users/thisaccountdoesnotexist"), }) - suite.EqualError(err, "GetRemoteAccount: couldn't retrieve account locally and won't try to resolve it") + var errNotRetrievable *dereferencing.ErrNotRetrievable + suite.ErrorAs(err, &errNotRetrievable) + suite.EqualError(err, "item could not be retrieved: GetRemoteAccount: couldn't retrieve account locally and not allowed to resolve it") suite.Nil(fetchedAccount) } @@ -233,7 +239,7 @@ func (suite *AccountTestSuite) TestDereferenceRemoteAccountWithPartial() { }, } - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountID: testrig.URLMustParse(remoteAccount.URI), RemoteAccountHost: remoteAccount.Domain, @@ -286,7 +292,7 @@ func (suite *AccountTestSuite) TestDereferenceRemoteAccountWithPartial2() { }, } - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountID: testrig.URLMustParse(remoteAccount.URI), RemoteAccountHost: remoteAccount.Domain, @@ -339,7 +345,7 @@ func (suite *AccountTestSuite) TestDereferenceRemoteAccountWithPartial3() { }, } - fetchedAccount, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountID: testrig.URLMustParse(remoteAccount.URI), RemoteAccountHost: remoteAccount.Domain, @@ -386,7 +392,7 @@ func (suite *AccountTestSuite) TestDereferenceRemoteAccountWithPartial3() { }, } - fetchedAccount2, err := suite.dereferencer.GetRemoteAccount(context.Background(), dereferencing.GetRemoteAccountParams{ + fetchedAccount2, err := suite.dereferencer.GetAccount(context.Background(), dereferencing.GetAccountParams{ RequestingUsername: fetchingAccount.Username, RemoteAccountID: testrig.URLMustParse(remoteAccount.URI), RemoteAccountHost: remoteAccount.Domain, diff --git a/internal/federation/dereferencing/announce.go b/internal/federation/dereferencing/announce.go index 144ddcb13..82c3afda2 100644 --- a/internal/federation/dereferencing/announce.go +++ b/internal/federation/dereferencing/announce.go @@ -58,7 +58,7 @@ func (d *deref) DereferenceAnnounce(ctx context.Context, announce *gtsmodel.Stat boostedStatus = status } else { // This is a boost of a remote status, we need to dereference it. - status, statusable, err := d.GetRemoteStatus(ctx, requestingUsername, boostedURI, true, true) + status, statusable, err := d.GetStatus(ctx, requestingUsername, boostedURI, true, true) if err != nil { return fmt.Errorf("DereferenceAnnounce: error dereferencing remote status with id %s: %s", announce.BoostOf.URI, err) } diff --git a/internal/federation/dereferencing/dereferencer.go b/internal/federation/dereferencing/dereferencer.go index f043e7cbd..a5503f359 100644 --- a/internal/federation/dereferencing/dereferencer.go +++ b/internal/federation/dereferencing/dereferencer.go @@ -33,19 +33,17 @@ import ( // Dereferencer wraps logic and functionality for doing dereferencing of remote accounts, statuses, etc, from federated instances. type Dereferencer interface { - GetRemoteAccount(ctx context.Context, params GetRemoteAccountParams) (*gtsmodel.Account, error) + GetAccount(ctx context.Context, params GetAccountParams) (*gtsmodel.Account, error) + GetStatus(ctx context.Context, username string, remoteStatusID *url.URL, refetch, includeParent bool) (*gtsmodel.Status, ap.Statusable, error) - GetRemoteStatus(ctx context.Context, username string, remoteStatusID *url.URL, refetch, includeParent bool) (*gtsmodel.Status, ap.Statusable, error) EnrichRemoteStatus(ctx context.Context, username string, status *gtsmodel.Status, includeParent bool) (*gtsmodel.Status, error) - GetRemoteInstance(ctx context.Context, username string, remoteInstanceURI *url.URL) (*gtsmodel.Instance, error) + DereferenceAnnounce(ctx context.Context, announce *gtsmodel.Status, requestingUsername string) error + DereferenceThread(ctx context.Context, username string, statusIRI *url.URL, status *gtsmodel.Status, statusable ap.Statusable) GetRemoteMedia(ctx context.Context, requestingUsername string, accountID string, remoteURL string, ai *media.AdditionalMediaInfo) (*media.ProcessingMedia, error) GetRemoteEmoji(ctx context.Context, requestingUsername string, remoteURL string, shortcode string, domain string, id string, emojiURI string, ai *media.AdditionalEmojiInfo, refresh bool) (*media.ProcessingEmoji, error) - DereferenceAnnounce(ctx context.Context, announce *gtsmodel.Status, requestingUsername string) error - DereferenceThread(ctx context.Context, username string, statusIRI *url.URL, status *gtsmodel.Status, statusable ap.Statusable) - Handshaking(ctx context.Context, username string, remoteAccountID *url.URL) bool } diff --git a/internal/federation/dereferencing/error.go b/internal/federation/dereferencing/error.go new file mode 100644 index 000000000..4e5d1cd22 --- /dev/null +++ b/internal/federation/dereferencing/error.go @@ -0,0 +1,132 @@ +/* + GoToSocial + Copyright (C) 2021-2022 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 dereferencing + +import ( + "errors" + "fmt" + + "github.com/superseriousbusiness/gotosocial/internal/transport" +) + +// ErrDB denotes that a proper error has occurred when doing +// a database call, as opposed to a simple db.ErrNoEntries. +type ErrDB struct { + wrapped error +} + +func (err *ErrDB) Error() string { + return fmt.Sprintf("database error during dereferencing: %v", err.wrapped) +} + +func newErrDB(err error) error { + return &ErrDB{wrapped: err} +} + +// ErrNotRetrievable denotes that an item could not be dereferenced +// with the given parameters. +type ErrNotRetrievable struct { + wrapped error +} + +func (err *ErrNotRetrievable) Error() string { + return fmt.Sprintf("item could not be retrieved: %v", err.wrapped) +} + +func newErrNotRetrievable(err error) error { + return &ErrNotRetrievable{wrapped: err} +} + +// ErrBadRequest denotes that insufficient or improperly formed parameters +// were passed into one of the dereference functions. +type ErrBadRequest struct { + wrapped error +} + +func (err *ErrBadRequest) Error() string { + return fmt.Sprintf("bad request: %v", err.wrapped) +} + +func newErrBadRequest(err error) error { + return &ErrBadRequest{wrapped: err} +} + +// ErrTransportError indicates that something unforeseen went wrong creating +// a transport, or while making an http call to a remote resource with a transport. +type ErrTransportError struct { + wrapped error +} + +func (err *ErrTransportError) Error() string { + return fmt.Sprintf("transport error: %v", err.wrapped) +} + +func newErrTransportError(err error) error { + return &ErrTransportError{wrapped: err} +} + +// ErrWrongType indicates that an unexpected type was returned from a remote call; +// for example, we were served a Person when we were looking for a statusable. +type ErrWrongType struct { + wrapped error +} + +func (err *ErrWrongType) Error() string { + return fmt.Sprintf("wrong received type: %v", err.wrapped) +} + +func newErrWrongType(err error) error { + return &ErrWrongType{wrapped: err} +} + +// ErrOther denotes some other kind of weird error, perhaps from a malformed json +// or some other weird crapola. +type ErrOther struct { + wrapped error +} + +func (err *ErrOther) Error() string { + return fmt.Sprintf("unexpected error: %v", err.wrapped) +} + +func newErrOther(err error) error { + return &ErrOther{wrapped: err} +} + +func wrapDerefError(derefErr error, fluff string) error { + var ( + err error + errWrongType *ErrWrongType + ) + + if fluff != "" { + err = fmt.Errorf("%s: %w", fluff, derefErr) + } + + switch { + case errors.Is(derefErr, transport.ErrGone): + err = newErrNotRetrievable(err) + case errors.As(derefErr, &errWrongType): + err = newErrWrongType(err) + default: + err = newErrTransportError(err) + } + + return err +} diff --git a/internal/federation/dereferencing/status.go b/internal/federation/dereferencing/status.go index 001fe53f4..a1783e838 100644 --- a/internal/federation/dereferencing/status.go +++ b/internal/federation/dereferencing/status.go @@ -29,6 +29,7 @@ import ( "github.com/superseriousbusiness/activity/streams" "github.com/superseriousbusiness/activity/streams/vocab" "github.com/superseriousbusiness/gotosocial/internal/ap" + "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/id" @@ -36,7 +37,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/media" ) -// EnrichRemoteStatus takes a status that's already been inserted into the database in a minimal form, +// EnrichRemoteStatus takes a remote status that's already been inserted into the database in a minimal form, // and populates it with additional fields, media, etc. // // EnrichRemoteStatus is mostly useful for calling after a status has been initially created by @@ -51,7 +52,7 @@ func (d *deref) EnrichRemoteStatus(ctx context.Context, username string, status return status, nil } -// GetRemoteStatus completely dereferences a remote status, converts it to a GtS model status, +// GetStatus completely dereferences a status, converts it to a GtS model status, // puts it in the database, and returns it to a caller. // // If refetch is true, then regardless of whether we have the original status in the database or not, @@ -60,58 +61,96 @@ func (d *deref) EnrichRemoteStatus(ctx context.Context, username string, status // If refetch is false, the ap.Statusable will only be returned if this is a new status, so callers // should check whether or not this is nil. // -// SIDE EFFECTS: remote status will be stored in the database, and the remote status owner will also be stored. -func (d *deref) GetRemoteStatus(ctx context.Context, username string, remoteStatusID *url.URL, refetch, includeParent bool) (*gtsmodel.Status, ap.Statusable, error) { - maybeStatus, err := d.db.GetStatusByURI(ctx, remoteStatusID.String()) - if err == nil && !refetch { +// GetAccount will guard against trying to do http calls to fetch a status that belongs to this instance. +// Instead of making calls, it will just return the status early if it finds it, or return an error. +func (d *deref) GetStatus(ctx context.Context, username string, statusURI *url.URL, refetch, includeParent bool) (*gtsmodel.Status, ap.Statusable, error) { + uriString := statusURI.String() + + // try to get by URI first + status, dbErr := d.db.GetStatusByURI(ctx, uriString) + if dbErr != nil { + if !errors.Is(dbErr, db.ErrNoEntries) { + // real error + return nil, nil, newErrDB(fmt.Errorf("GetRemoteStatus: error during GetStatusByURI for %s: %w", uriString, dbErr)) + } + // no problem, just press on + } else if !refetch { // we already had the status and we aren't being asked to refetch the AP representation - return maybeStatus, nil, nil + return status, nil, nil } - statusable, err := d.dereferenceStatusable(ctx, username, remoteStatusID) - if err != nil { - return nil, nil, fmt.Errorf("GetRemoteStatus: error dereferencing statusable: %s", err) + // try to get by URL if we couldn't get by URI now + if status == nil { + status, dbErr = d.db.GetStatusByURL(ctx, uriString) + if dbErr != nil { + if !errors.Is(dbErr, db.ErrNoEntries) { + // real error + return nil, nil, newErrDB(fmt.Errorf("GetRemoteStatus: error during GetStatusByURI for %s: %w", uriString, dbErr)) + } + // no problem, just press on + } else if !refetch { + // we already had the status and we aren't being asked to refetch the AP representation + return status, nil, nil + } } - if maybeStatus != nil && refetch { - // we already had the status and we've successfully fetched the AP representation as requested - return maybeStatus, statusable, nil + // guard against having our own statuses passed in + if host := statusURI.Host; host == config.GetHost() || host == config.GetAccountDomain() { + // this is our status, definitely don't search for it + if status != nil { + return status, nil, nil + } + return nil, nil, newErrNotRetrievable(fmt.Errorf("GetRemoteStatus: uri %s is apparently ours, but we have nothing in the db for it, will not proceed to dereference our own status", uriString)) + } + + // if we got here, either we didn't have the status + // in the db, or we had it but need to refetch it + statusable, derefErr := d.dereferenceStatusable(ctx, username, statusURI) + if derefErr != nil { + return nil, nil, wrapDerefError(derefErr, "GetRemoteStatus: error dereferencing statusable") + } + + if status != nil && refetch { + // we already had the status in the db, and we've also + // now fetched the AP representation as requested + return status, statusable, nil } // from here on out we can consider this to be a 'new' status because we didn't have the status in the db already accountURI, err := ap.ExtractAttributedTo(statusable) if err != nil { - return nil, nil, fmt.Errorf("GetRemoteStatus: error extracting attributedTo: %s", err) + return nil, nil, newErrOther(fmt.Errorf("GetRemoteStatus: error extracting attributedTo: %w", err)) } - _, err = d.GetRemoteAccount(ctx, GetRemoteAccountParams{ + // we need to get the author of the status else we can't serialize it properly + if _, err = d.GetAccount(ctx, GetAccountParams{ RequestingUsername: username, RemoteAccountID: accountURI, - }) + Blocking: true, + }); err != nil { + return nil, nil, newErrOther(fmt.Errorf("GetRemoteStatus: couldn't get status author: %s", err)) + } + + status, err = d.typeConverter.ASStatusToStatus(ctx, statusable) if err != nil { - return nil, nil, fmt.Errorf("GetRemoteStatus: couldn't get status author: %s", err) + return nil, nil, newErrOther(fmt.Errorf("GetRemoteStatus: error converting statusable to status: %s", err)) } - gtsStatus, err := d.typeConverter.ASStatusToStatus(ctx, statusable) + ulid, err := id.NewULIDFromTime(status.CreatedAt) if err != nil { - return nil, statusable, fmt.Errorf("GetRemoteStatus: error converting statusable to status: %s", err) + return nil, nil, newErrOther(fmt.Errorf("GetRemoteStatus: error generating new id for status: %s", err)) + } + status.ID = ulid + + if err := d.populateStatusFields(ctx, status, username, includeParent); err != nil { + return nil, nil, newErrOther(fmt.Errorf("GetRemoteStatus: error populating status fields: %s", err)) } - ulid, err := id.NewULIDFromTime(gtsStatus.CreatedAt) - if err != nil { - return nil, nil, fmt.Errorf("GetRemoteStatus: error generating new id for status: %s", err) - } - gtsStatus.ID = ulid - - if err := d.populateStatusFields(ctx, gtsStatus, username, includeParent); err != nil { - return nil, nil, fmt.Errorf("GetRemoteStatus: error populating status fields: %s", err) + if err := d.db.PutStatus(ctx, status); err != nil && !errors.Is(err, db.ErrAlreadyExists) { + return nil, nil, newErrDB(fmt.Errorf("GetRemoteStatus: error putting new status: %s", err)) } - if err := d.db.PutStatus(ctx, gtsStatus); err != nil && !errors.Is(err, db.ErrAlreadyExists) { - return nil, nil, fmt.Errorf("GetRemoteStatus: error putting new status: %s", err) - } - - return gtsStatus, statusable, nil + return status, statusable, nil } func (d *deref) dereferenceStatusable(ctx context.Context, username string, remoteStatusID *url.URL) (ap.Statusable, error) { @@ -197,7 +236,7 @@ func (d *deref) dereferenceStatusable(ctx context.Context, username string, remo return p, nil } - return nil, fmt.Errorf("DereferenceStatusable: type name %s not supported", t.GetTypeName()) + return nil, newErrWrongType(fmt.Errorf("DereferenceStatusable: type name %s not supported as Statusable", t.GetTypeName())) } // populateStatusFields fetches all the information we temporarily pinned to an incoming @@ -314,7 +353,7 @@ func (d *deref) populateStatusMentions(ctx context.Context, status *gtsmodel.Sta if targetAccount == nil { // we didn't find the account in our database already // check if we can get the account remotely (dereference it) - if a, err := d.GetRemoteAccount(ctx, GetRemoteAccountParams{ + if a, err := d.GetAccount(ctx, GetAccountParams{ RequestingUsername: requestingUsername, RemoteAccountID: targetAccountURI, }); err != nil { @@ -430,7 +469,7 @@ func (d *deref) populateStatusRepliedTo(ctx context.Context, status *gtsmodel.St return err } - replyToStatus, _, err := d.GetRemoteStatus(ctx, requestingUsername, statusURI, false, false) + replyToStatus, _, err := d.GetStatus(ctx, requestingUsername, statusURI, false, false) if err != nil { return fmt.Errorf("populateStatusRepliedTo: couldn't get reply to status with uri %s: %s", status.InReplyToURI, err) } diff --git a/internal/federation/dereferencing/status_test.go b/internal/federation/dereferencing/status_test.go index 964de23ec..da6fe3be6 100644 --- a/internal/federation/dereferencing/status_test.go +++ b/internal/federation/dereferencing/status_test.go @@ -37,7 +37,7 @@ func (suite *StatusTestSuite) TestDereferenceSimpleStatus() { fetchingAccount := suite.testAccounts["local_account_1"] statusURL := testrig.URLMustParse("https://unknown-instance.com/users/brand_new_person/statuses/01FE4NTHKWW7THT67EF10EB839") - status, _, err := suite.dereferencer.GetRemoteStatus(context.Background(), fetchingAccount.Username, statusURL, false, false) + status, _, err := suite.dereferencer.GetStatus(context.Background(), fetchingAccount.Username, statusURL, false, false) suite.NoError(err) suite.NotNil(status) @@ -77,7 +77,7 @@ func (suite *StatusTestSuite) TestDereferenceStatusWithMention() { fetchingAccount := suite.testAccounts["local_account_1"] statusURL := testrig.URLMustParse("https://unknown-instance.com/users/brand_new_person/statuses/01FE5Y30E3W4P7TRE0R98KAYQV") - status, _, err := suite.dereferencer.GetRemoteStatus(context.Background(), fetchingAccount.Username, statusURL, false, false) + status, _, err := suite.dereferencer.GetStatus(context.Background(), fetchingAccount.Username, statusURL, false, false) suite.NoError(err) suite.NotNil(status) @@ -128,7 +128,7 @@ func (suite *StatusTestSuite) TestDereferenceStatusWithImageAndNoContent() { fetchingAccount := suite.testAccounts["local_account_1"] statusURL := testrig.URLMustParse("https://turnip.farm/users/turniplover6969/statuses/70c53e54-3146-42d5-a630-83c8b6c7c042") - status, _, err := suite.dereferencer.GetRemoteStatus(context.Background(), fetchingAccount.Username, statusURL, false, false) + status, _, err := suite.dereferencer.GetStatus(context.Background(), fetchingAccount.Username, statusURL, false, false) suite.NoError(err) suite.NotNil(status) diff --git a/internal/federation/dereferencing/thread.go b/internal/federation/dereferencing/thread.go index 34955cee8..15829c45a 100644 --- a/internal/federation/dereferencing/thread.go +++ b/internal/federation/dereferencing/thread.go @@ -114,7 +114,7 @@ func (d *deref) dereferenceStatusAncestors(ctx context.Context, username string, l.Tracef("following remote status ancestors: %s", status.InReplyToURI) // Fetch the remote status found at this IRI - remoteStatus, _, err := d.GetRemoteStatus(ctx, username, replyIRI, false, false) + remoteStatus, _, err := d.GetStatus(ctx, username, replyIRI, false, false) if err != nil { return fmt.Errorf("error fetching remote status %q: %w", status.InReplyToURI, err) } @@ -276,7 +276,7 @@ stackLoop: } // Dereference the remote status and store in the database - _, statusable, err := d.GetRemoteStatus(ctx, username, itemIRI, true, false) + _, statusable, err := d.GetStatus(ctx, username, itemIRI, true, false) if err != nil { l.Errorf("error dereferencing remote status %q: %s", itemIRI.String(), err) continue itemLoop diff --git a/internal/federation/federatingprotocol.go b/internal/federation/federatingprotocol.go index dbec47f3d..e7a897c83 100644 --- a/internal/federation/federatingprotocol.go +++ b/internal/federation/federatingprotocol.go @@ -206,7 +206,7 @@ func (f *federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr } } - requestingAccount, err := f.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + requestingAccount, err := f.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: username, RemoteAccountID: publicKeyOwnerURI, }) diff --git a/internal/federation/federator.go b/internal/federation/federator.go index 0b09144c2..bf847db0a 100644 --- a/internal/federation/federator.go +++ b/internal/federation/federator.go @@ -53,14 +53,14 @@ type Federator interface { // If something goes wrong during authentication, nil, false, and an error will be returned. AuthenticateFederatedRequest(ctx context.Context, username string) (*url.URL, gtserror.WithCode) + /* + dereferencing functions + */ DereferenceRemoteThread(ctx context.Context, username string, statusURI *url.URL, status *gtsmodel.Status, statusable ap.Statusable) DereferenceAnnounce(ctx context.Context, announce *gtsmodel.Status, requestingUsername string) error - - GetRemoteAccount(ctx context.Context, params dereferencing.GetRemoteAccountParams) (*gtsmodel.Account, error) - - GetRemoteStatus(ctx context.Context, username string, remoteStatusID *url.URL, refetch, includeParent bool) (*gtsmodel.Status, ap.Statusable, error) + GetAccount(ctx context.Context, params dereferencing.GetAccountParams) (*gtsmodel.Account, error) + GetStatus(ctx context.Context, username string, remoteStatusID *url.URL, refetch, includeParent bool) (*gtsmodel.Status, ap.Statusable, error) EnrichRemoteStatus(ctx context.Context, username string, status *gtsmodel.Status, includeParent bool) (*gtsmodel.Status, error) - GetRemoteInstance(ctx context.Context, username string, remoteInstanceURI *url.URL) (*gtsmodel.Instance, error) // Handshaking returns true if the given username is currently in the process of dereferencing the remoteAccountID. diff --git a/internal/processing/account/get.go b/internal/processing/account/get.go index 2971dcd2b..503aa9654 100644 --- a/internal/processing/account/get.go +++ b/internal/processing/account/get.go @@ -94,7 +94,7 @@ func (p *processor) getAccountFor(ctx context.Context, requestingAccount *gtsmod return nil, gtserror.NewErrorInternalError(fmt.Errorf("error parsing url %s: %s", targetAccount.URI, err)) } - a, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + a, err := p.federator.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: requestingAccount.Username, RemoteAccountID: targetAccountURI, RemoteAccountHost: targetAccount.Domain, diff --git a/internal/processing/federation/getfollowers.go b/internal/processing/federation/getfollowers.go index 22cffdc73..75636c5a0 100644 --- a/internal/processing/federation/getfollowers.go +++ b/internal/processing/federation/getfollowers.go @@ -42,7 +42,7 @@ func (p *processor) GetFollowers(ctx context.Context, requestedUsername string, return nil, errWithCode } - requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/federation/getfollowing.go b/internal/processing/federation/getfollowing.go index c1c7d208e..673f47562 100644 --- a/internal/processing/federation/getfollowing.go +++ b/internal/processing/federation/getfollowing.go @@ -42,7 +42,7 @@ func (p *processor) GetFollowing(ctx context.Context, requestedUsername string, return nil, errWithCode } - requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/federation/getoutbox.go b/internal/processing/federation/getoutbox.go index 67e7f65d7..55a6a6339 100644 --- a/internal/processing/federation/getoutbox.go +++ b/internal/processing/federation/getoutbox.go @@ -43,7 +43,7 @@ func (p *processor) GetOutbox(ctx context.Context, requestedUsername string, pag return nil, errWithCode } - requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/federation/getstatus.go b/internal/processing/federation/getstatus.go index bbd86b74c..286cce3e0 100644 --- a/internal/processing/federation/getstatus.go +++ b/internal/processing/federation/getstatus.go @@ -42,7 +42,7 @@ func (p *processor) GetStatus(ctx context.Context, requestedUsername string, req return nil, errWithCode } - requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/federation/getstatusreplies.go b/internal/processing/federation/getstatusreplies.go index 9575a7b36..134be54e7 100644 --- a/internal/processing/federation/getstatusreplies.go +++ b/internal/processing/federation/getstatusreplies.go @@ -44,7 +44,7 @@ func (p *processor) GetStatusReplies(ctx context.Context, requestedUsername stri return nil, errWithCode } - requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/federation/getuser.go b/internal/processing/federation/getuser.go index f203d40c8..c1439cdff 100644 --- a/internal/processing/federation/getuser.go +++ b/internal/processing/federation/getuser.go @@ -54,7 +54,7 @@ func (p *processor) GetUser(ctx context.Context, requestedUsername string, reque // if we're not already handshaking/dereferencing a remote account, dereference it now if !p.federator.Handshaking(ctx, requestedUsername, requestingAccountURI) { - requestingAccount, err := p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + requestingAccount, err := p.federator.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: requestedUsername, RemoteAccountID: requestingAccountURI, }) diff --git a/internal/processing/fromfederator.go b/internal/processing/fromfederator.go index 29d996502..e44031eeb 100644 --- a/internal/processing/fromfederator.go +++ b/internal/processing/fromfederator.go @@ -129,7 +129,7 @@ func (p *processor) processCreateStatusFromFederator(ctx context.Context, federa return errors.New("ProcessFromFederator: status was not pinned to federatorMsg, and neither was an IRI for us to dereference") } var err error - status, _, err = p.federator.GetRemoteStatus(ctx, federatorMsg.ReceivingAccount.Username, federatorMsg.APIri, false, false) + status, _, err = p.federator.GetStatus(ctx, federatorMsg.ReceivingAccount.Username, federatorMsg.APIri, false, false) if err != nil { return err } @@ -151,7 +151,7 @@ func (p *processor) processCreateStatusFromFederator(ctx context.Context, federa return err } - a, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + a, err := p.federator.GetAccount(ctx, dereferencing.GetAccountParams{ RequestingUsername: federatorMsg.ReceivingAccount.Username, RemoteAccountID: remoteAccountID, Blocking: true, @@ -197,7 +197,7 @@ func (p *processor) processCreateFaveFromFederator(ctx context.Context, federato return err } - a, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + a, err := p.federator.GetAccount(ctx, dereferencing.GetAccountParams{ RequestingUsername: federatorMsg.ReceivingAccount.Username, RemoteAccountID: remoteAccountID, Blocking: true, @@ -239,7 +239,7 @@ func (p *processor) processCreateFollowRequestFromFederator(ctx context.Context, return err } - a, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + a, err := p.federator.GetAccount(ctx, dereferencing.GetAccountParams{ RequestingUsername: federatorMsg.ReceivingAccount.Username, RemoteAccountID: remoteAccountID, Blocking: true, @@ -300,7 +300,7 @@ func (p *processor) processCreateAnnounceFromFederator(ctx context.Context, fede return err } - a, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + a, err := p.federator.GetAccount(ctx, dereferencing.GetAccountParams{ RequestingUsername: federatorMsg.ReceivingAccount.Username, RemoteAccountID: remoteAccountID, Blocking: true, @@ -370,7 +370,7 @@ func (p *processor) processUpdateAccountFromFederator(ctx context.Context, feder } // further database updates occur inside getremoteaccount - if _, err := p.federator.GetRemoteAccount(ctx, dereferencing.GetRemoteAccountParams{ + if _, err := p.federator.GetAccount(ctx, dereferencing.GetAccountParams{ RequestingUsername: federatorMsg.ReceivingAccount.Username, RemoteAccountID: incomingAccountURL, RemoteAccountHost: incomingAccount.Domain, diff --git a/internal/processing/search.go b/internal/processing/search.go index bc2bc93d4..ca6cc42ce 100644 --- a/internal/processing/search.go +++ b/internal/processing/search.go @@ -27,8 +27,6 @@ import ( "codeberg.org/gruf/go-kv" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" - "github.com/superseriousbusiness/gotosocial/internal/config" - "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" @@ -38,11 +36,18 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/util" ) +// Implementation note: in this function, we tend to log errors +// at debug level rather than return them. This is because the +// search has a sort of fallthrough logic: if we can't get a result +// with x search, we should try with y search rather than returning. +// +// If we get to the end and still haven't found anything, even then +// we shouldn't return an error, just return an empty search result. +// +// The only exception to this is when we get a malformed query, in +// which case we return a bad request error so the user knows they +// did something funky. func (p *processor) SearchGet(ctx context.Context, authed *oauth.Auth, search *apimodel.SearchQuery) (*apimodel.SearchResult, gtserror.WithCode) { - l := log.WithFields(kv.Fields{ - {"query", search.Query}, - }...) - // tidy up the query and make sure it wasn't just spaces query := strings.TrimSpace(search.Query) if query == "" { @@ -50,6 +55,8 @@ func (p *processor) SearchGet(ctx context.Context, authed *oauth.Auth, search *a return nil, gtserror.NewErrorBadRequest(err, err.Error()) } + l := log.WithFields(kv.Fields{{"query", query}}...) + searchResult := &apimodel.SearchResult{ Accounts: []apimodel.Account{}, Statuses: []apimodel.Status{}, @@ -77,14 +84,20 @@ func (p *processor) SearchGet(ctx context.Context, authed *oauth.Auth, search *a } if username, domain, err := util.ExtractNamestringParts(maybeNamestring); err == nil { - l.Debugf("search term %s is a mention, looking it up...", maybeNamestring) - if foundAccount, err := p.searchAccountByMention(ctx, authed, username, domain, search.Resolve); err == nil && foundAccount != nil { - foundAccounts = append(foundAccounts, foundAccount) - foundOne = true - l.Debug("got an account by searching by mention") - } else if err != nil { - l.Debugf("error looking up account %s: %s", maybeNamestring, err) + l.Trace("search term is a mention, looking it up...") + foundAccount, err := p.searchAccountByMention(ctx, authed, username, domain, search.Resolve) + if err != nil { + var errNotRetrievable *dereferencing.ErrNotRetrievable + if !errors.As(err, &errNotRetrievable) { + // return a proper error only if it wasn't just not retrievable + return nil, gtserror.NewErrorInternalError(fmt.Errorf("error looking up account: %w", err)) + } + return searchResult, nil } + + foundAccounts = append(foundAccounts, foundAccount) + foundOne = true + l.Trace("got an account by searching by mention") } /* @@ -92,46 +105,95 @@ func (p *processor) SearchGet(ctx context.Context, authed *oauth.Auth, search *a check if the query is a URI with a recognizable scheme and dereference it */ if !foundOne { - if uri, err := url.Parse(query); err == nil && (uri.Scheme == "https" || uri.Scheme == "http") { - // don't attempt to resolve (ie., dereference) local accounts/statuses - resolve := search.Resolve - if uri.Host == config.GetHost() || uri.Host == config.GetAccountDomain() { - resolve = false - } + if uri, err := url.Parse(query); err == nil { + if uri.Scheme == "https" || uri.Scheme == "http" { + l.Trace("search term is a uri, looking it up...") + // check if it's a status... + foundStatus, err := p.searchStatusByURI(ctx, authed, uri) + if err != nil { + var ( + errNotRetrievable *dereferencing.ErrNotRetrievable + errWrongType *dereferencing.ErrWrongType + ) + if !errors.As(err, &errNotRetrievable) && !errors.As(err, &errWrongType) { + return nil, gtserror.NewErrorInternalError(fmt.Errorf("error looking up status: %w", err)) + } + } else { + foundStatuses = append(foundStatuses, foundStatus) + foundOne = true + l.Trace("got a status by searching by URI") + } - // check if it's a status or an account - if foundStatus, err := p.searchStatusByURI(ctx, authed, uri, resolve); err == nil && foundStatus != nil { - foundStatuses = append(foundStatuses, foundStatus) - l.Debug("got a status by searching by URI") - } else if foundAccount, err := p.searchAccountByURI(ctx, authed, uri, resolve); err == nil && foundAccount != nil { - foundAccounts = append(foundAccounts, foundAccount) - l.Debug("got an account by searching by URI") + // ... or an account + if !foundOne { + foundAccount, err := p.searchAccountByURI(ctx, authed, uri, search.Resolve) + if err != nil { + var ( + errNotRetrievable *dereferencing.ErrNotRetrievable + errWrongType *dereferencing.ErrWrongType + ) + if !errors.As(err, &errNotRetrievable) && !errors.As(err, &errWrongType) { + return nil, gtserror.NewErrorInternalError(fmt.Errorf("error looking up account: %w", err)) + } + } else { + foundAccounts = append(foundAccounts, foundAccount) + foundOne = true + l.Trace("got an account by searching by URI") + } + } } } } + if !foundOne { + // we got nothing, we can return early + l.Trace("found nothing, returning") + return searchResult, nil + } + /* FROM HERE ON we have our search results, it's just a matter of filtering them according to what this user is allowed to see, and then converting them into our frontend format. */ for _, foundAccount := range foundAccounts { // make sure there's no block in either direction between the account and the requester - if blocked, err := p.db.IsBlocked(ctx, authed.Account.ID, foundAccount.ID, true); err == nil && !blocked { - // all good, convert it and add it to the results - if apiAcct, err := p.tc.AccountToAPIAccountPublic(ctx, foundAccount); err == nil && apiAcct != nil { - searchResult.Accounts = append(searchResult.Accounts, *apiAcct) - } + blocked, err := p.db.IsBlocked(ctx, authed.Account.ID, foundAccount.ID, true) + if err != nil { + err = fmt.Errorf("SearchGet: error checking block between %s and %s: %s", authed.Account.ID, foundAccount.ID, err) + return nil, gtserror.NewErrorInternalError(err) } + + if blocked { + l.Tracef("block exists between %s and %s, skipping this result", authed.Account.ID, foundAccount.ID) + continue + } + + apiAcct, err := p.tc.AccountToAPIAccountPublic(ctx, foundAccount) + if err != nil { + err = fmt.Errorf("SearchGet: error converting account %s to api account: %s", foundAccount.ID, err) + return nil, gtserror.NewErrorInternalError(err) + } + + searchResult.Accounts = append(searchResult.Accounts, *apiAcct) } for _, foundStatus := range foundStatuses { - if visible, err := p.filter.StatusVisible(ctx, foundStatus, authed.Account); !visible || err != nil { + // make sure each found status is visible to the requester + visible, err := p.filter.StatusVisible(ctx, foundStatus, authed.Account) + if err != nil { + err = fmt.Errorf("SearchGet: error checking visibility of status %s for account %s: %s", foundStatus.ID, authed.Account.ID, err) + return nil, gtserror.NewErrorInternalError(err) + } + + if !visible { + l.Tracef("status %s is not visible to account %s, skipping this result", foundStatus.ID, authed.Account.ID) continue } apiStatus, err := p.tc.StatusToAPIStatus(ctx, foundStatus, authed.Account) if err != nil { - continue + err = fmt.Errorf("SearchGet: error converting status %s to api status: %s", foundStatus.ID, err) + return nil, gtserror.NewErrorInternalError(err) } searchResult.Statuses = append(searchResult.Statuses, *apiStatus) @@ -140,58 +202,22 @@ func (p *processor) SearchGet(ctx context.Context, authed *oauth.Auth, search *a return searchResult, nil } -func (p *processor) searchStatusByURI(ctx context.Context, authed *oauth.Auth, uri *url.URL, resolve bool) (*gtsmodel.Status, error) { - // Calculate URI string once - uriStr := uri.String() - - // Look for status locally (by URI), we only accept "not found" errors. - status, err := p.db.GetStatusByURI(ctx, uriStr) - if err != nil && !errors.Is(err, db.ErrNoEntries) { - return nil, fmt.Errorf("searchStatusByURI: error fetching status %q: %v", uriStr, err) - } else if err == nil { - return status, nil +func (p *processor) searchStatusByURI(ctx context.Context, authed *oauth.Auth, uri *url.URL) (*gtsmodel.Status, error) { + status, statusable, err := p.federator.GetStatus(transport.WithFastfail(ctx), authed.Account.Username, uri, true, true) + if err != nil { + return nil, err } - // Again, look for status locally (by URL), we only accept "not found" errors. - status, err = p.db.GetStatusByURL(ctx, uriStr) - if err != nil && !errors.Is(err, db.ErrNoEntries) { - return nil, fmt.Errorf("searchStatusByURI: error fetching status %q: %v", uriStr, err) - } else if err == nil { - return status, nil - } - - if resolve { - // This is a non-local status and we're allowed to resolve, so dereference it - status, statusable, err := p.federator.GetRemoteStatus(transport.WithFastfail(ctx), authed.Account.Username, uri, true, true) - if err != nil { - return nil, fmt.Errorf("searchStatusByURI: error fetching remote status %q: %v", uriStr, err) - } - + if !*status.Local && statusable != nil { // Attempt to dereference the status thread while we are here p.federator.DereferenceRemoteThread(transport.WithFastfail(ctx), authed.Account.Username, uri, status, statusable) } - return nil, nil + return status, nil } func (p *processor) searchAccountByURI(ctx context.Context, authed *oauth.Auth, uri *url.URL, resolve bool) (*gtsmodel.Account, error) { - // it might be a web url like http://example.org/@user instead - // of an AP uri like http://example.org/users/user, check first - if maybeAccount, err := p.db.GetAccountByURL(ctx, uri.String()); err == nil { - return maybeAccount, nil - } - - if uri.Host == config.GetHost() || uri.Host == config.GetAccountDomain() { - // this is a local account; if we don't have it now then - // we should just bail instead of trying to get it remote - if maybeAccount, err := p.db.GetAccountByURI(ctx, uri.String()); err == nil { - return maybeAccount, nil - } - return nil, nil - } - - // we don't have it yet, try to find it remotely - return p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + return p.federator.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: authed.Account.Username, RemoteAccountID: uri, Blocking: true, @@ -200,17 +226,7 @@ func (p *processor) searchAccountByURI(ctx context.Context, authed *oauth.Auth, } func (p *processor) searchAccountByMention(ctx context.Context, authed *oauth.Auth, username string, domain string, resolve bool) (*gtsmodel.Account, error) { - // if it's a local account we can skip a whole bunch of stuff - if domain == config.GetHost() || domain == config.GetAccountDomain() || domain == "" { - maybeAcct, err := p.db.GetAccountByUsernameDomain(ctx, username, "") - if err == nil || err == db.ErrNoEntries { - return maybeAcct, nil - } - return nil, fmt.Errorf("searchAccountByMention: error getting local account by username: %s", err) - } - - // we don't have it yet, try to find it remotely - return p.federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + return p.federator.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: authed.Account.Username, RemoteAccountUsername: username, RemoteAccountHost: domain, diff --git a/internal/processing/util.go b/internal/processing/util.go index 8b1399d3c..b3d9416c4 100644 --- a/internal/processing/util.go +++ b/internal/processing/util.go @@ -57,7 +57,7 @@ func GetParseMentionFunc(dbConn db.DB, federator federation.Federator) gtsmodel. if originAccount.Domain == "" { requestingUsername = originAccount.Username } - remoteAccount, err := federator.GetRemoteAccount(transport.WithFastfail(ctx), dereferencing.GetRemoteAccountParams{ + remoteAccount, err := federator.GetAccount(transport.WithFastfail(ctx), dereferencing.GetAccountParams{ RequestingUsername: requestingUsername, RemoteAccountUsername: username, RemoteAccountHost: domain,