summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLibravatar tobi <31960611+tsmethurst@users.noreply.github.com>2023-12-01 15:27:15 +0100
committerLibravatar GitHub <noreply@github.com>2023-12-01 15:27:15 +0100
commit0e2c34219112db3a6b7801530a946fd5b1bbb111 (patch)
tree6a5557373dbfc9edc80de941b13e870a8af32881
parent[bugfix] in fedi API CreateStatus(), handle case of data-race and return earl... (diff)
downloadgotosocial-0e2c34219112db3a6b7801530a946fd5b1bbb111.tar.xz
[bugfix/chore] `Announce` reliability updates (#2405)v0.13.0-rc1
* [bugfix/chore] `Announce` updates * test update * fix tests * TestParseAnnounce * update comments * don't lock/unlock, change function signature * naming stuff * don't check domain block twice * UnwrapIfBoost * beep boop
-rw-r--r--internal/federation/dereferencing/announce.go125
-rw-r--r--internal/federation/federatingdb/announce_test.go12
-rw-r--r--internal/gtsmodel/status.go1
-rw-r--r--internal/processing/common/status.go19
-rw-r--r--internal/processing/fedi/status.go10
-rw-r--r--internal/processing/status/boost.go161
-rw-r--r--internal/processing/status/fave.go32
-rw-r--r--internal/processing/workers/fromfediapi.go38
-rw-r--r--internal/processing/workers/fromfediapi_test.go4
-rw-r--r--internal/typeutils/astointernal.go83
-rw-r--r--internal/typeutils/astointernal_test.go40
-rw-r--r--internal/typeutils/internal.go122
-rw-r--r--internal/typeutils/internaltoas_test.go3
-rw-r--r--internal/typeutils/internaltofrontend.go54
-rw-r--r--internal/web/thread.go8
15 files changed, 426 insertions, 286 deletions
diff --git a/internal/federation/dereferencing/announce.go b/internal/federation/dereferencing/announce.go
index 22c33685a..8e880dad5 100644
--- a/internal/federation/dereferencing/announce.go
+++ b/internal/federation/dereferencing/announce.go
@@ -20,66 +20,107 @@ package dereferencing
import (
"context"
"errors"
- "fmt"
"net/url"
"github.com/superseriousbusiness/gotosocial/internal/config"
+ "github.com/superseriousbusiness/gotosocial/internal/db"
+ "github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
+ "github.com/superseriousbusiness/gotosocial/internal/id"
)
-func (d *Dereferencer) DereferenceAnnounce(ctx context.Context, announce *gtsmodel.Status, requestingUsername string) error {
- if announce.BoostOf == nil {
- // we can't do anything unfortunately
- return errors.New("DereferenceAnnounce: no URI to dereference")
+// EnrichAnnounce enriches the given boost wrapper status
+// by either fetching from the DB or dereferencing the target
+// status, populating the boost wrapper's fields based on the
+// target status, and then storing the wrapper in the database.
+// The wrapper is then returned to the caller.
+//
+// The provided boost wrapper status must have BoostOfURI set.
+func (d *Dereferencer) EnrichAnnounce(
+ ctx context.Context,
+ boost *gtsmodel.Status,
+ requestUser string,
+) (*gtsmodel.Status, error) {
+ targetURI := boost.BoostOfURI
+ if targetURI == "" {
+ // We can't do anything.
+ return nil, gtserror.Newf("no URI to dereference")
}
- // Parse the boosted status' URI
- boostedURI, err := url.Parse(announce.BoostOf.URI)
+ // Parse the boost target status URI.
+ targetURIObj, err := url.Parse(targetURI)
if err != nil {
- return fmt.Errorf("DereferenceAnnounce: couldn't parse boosted status URI %s: %s", announce.BoostOf.URI, err)
+ return nil, gtserror.Newf(
+ "couldn't parse boost target status URI %s: %w",
+ targetURI, err,
+ )
}
- // Check whether the originating status is from a blocked host
- if blocked, err := d.state.DB.IsDomainBlocked(ctx, boostedURI.Host); blocked || err != nil {
- return fmt.Errorf("DereferenceAnnounce: domain %s is blocked", boostedURI.Host)
+ // Fetch/deref status being boosted.
+ var target *gtsmodel.Status
+
+ if targetURIObj.Host == config.GetHost() {
+ // This is a local status, fetch from the database
+ target, err = d.state.DB.GetStatusByURI(ctx, targetURI)
+ } else {
+ // This is a remote status, we need to dereference it.
+ //
+ // d.GetStatusByURI will handle domain block checking for us,
+ // so we don't try to deref an announce target on a blocked host.
+ target, _, err = d.GetStatusByURI(ctx, requestUser, targetURIObj)
}
- var boostedStatus *gtsmodel.Status
+ if err != nil {
+ return nil, gtserror.Newf(
+ "error getting boost target status %s: %w",
+ targetURI, err,
+ )
+ }
- if boostedURI.Host == config.GetHost() {
- // This is a local status, fetch from the database
- status, err := d.state.DB.GetStatusByURI(ctx, boostedURI.String())
- if err != nil {
- return fmt.Errorf("DereferenceAnnounce: error fetching local status %q: %v", announce.BoostOf.URI, err)
- }
+ // Generate an ID for the boost wrapper status.
+ boost.ID, err = id.NewULIDFromTime(boost.CreatedAt)
+ if err != nil {
+ return nil, gtserror.Newf("error generating id: %w", err)
+ }
- // Set boosted status
- boostedStatus = status
- } else {
- // This is a boost of a remote status, we need to dereference it.
- status, _, err := d.GetStatusByURI(ctx, requestingUsername, boostedURI)
+ // Populate remaining fields on
+ // the boost wrapper using target.
+ boost.Content = target.Content
+ boost.ContentWarning = target.ContentWarning
+ boost.ActivityStreamsType = target.ActivityStreamsType
+ boost.Sensitive = target.Sensitive
+ boost.Language = target.Language
+ boost.Text = target.Text
+ boost.BoostOfID = target.ID
+ boost.BoostOf = target
+ boost.BoostOfAccountID = target.AccountID
+ boost.BoostOfAccount = target.Account
+ boost.Visibility = target.Visibility
+ boost.Federated = target.Federated
+ boost.Boostable = target.Boostable
+ boost.Replyable = target.Replyable
+ boost.Likeable = target.Likeable
+
+ // Store the boost wrapper status.
+ switch err = d.state.DB.PutStatus(ctx, boost); {
+ case err == nil:
+ // All good baby.
+
+ case errors.Is(err, db.ErrAlreadyExists):
+ // DATA RACE! We likely lost out to another goroutine
+ // in a call to db.Put(Status). Look again in DB by URI.
+ boost, err = d.state.DB.GetStatusByURI(ctx, boost.URI)
if err != nil {
- return fmt.Errorf("DereferenceAnnounce: error dereferencing remote status with id %s: %s", announce.BoostOf.URI, err)
+ err = gtserror.Newf(
+ "error getting boost wrapper status %s from database after race: %w",
+ boost.URI, err,
+ )
}
- // Set boosted status
- boostedStatus = status
+ default:
+ // Proper database error.
+ err = gtserror.Newf("db error inserting status: %w", err)
}
- announce.Content = boostedStatus.Content
- announce.ContentWarning = boostedStatus.ContentWarning
- announce.ActivityStreamsType = boostedStatus.ActivityStreamsType
- announce.Sensitive = boostedStatus.Sensitive
- announce.Language = boostedStatus.Language
- announce.Text = boostedStatus.Text
- announce.BoostOfID = boostedStatus.ID
- announce.BoostOfAccountID = boostedStatus.AccountID
- announce.Visibility = boostedStatus.Visibility
- announce.Federated = boostedStatus.Federated
- announce.Boostable = boostedStatus.Boostable
- announce.Replyable = boostedStatus.Replyable
- announce.Likeable = boostedStatus.Likeable
- announce.BoostOf = boostedStatus
-
- return nil
+ return boost, err
}
diff --git a/internal/federation/federatingdb/announce_test.go b/internal/federation/federatingdb/announce_test.go
index ae5213f66..d8de2e49c 100644
--- a/internal/federation/federatingdb/announce_test.go
+++ b/internal/federation/federatingdb/announce_test.go
@@ -50,8 +50,10 @@ func (suite *AnnounceTestSuite) TestNewAnnounce() {
suite.True(ok)
suite.Equal(announcingAccount.ID, boost.AccountID)
- // only the URI will be set on the boosted status because it still needs to be dereferenced
- suite.NotEmpty(boost.BoostOf.URI)
+ // only the URI will be set for the boosted status
+ // because it still needs to be dereferenced
+ suite.Nil(boost.BoostOf)
+ suite.Equal("http://example.org/users/Some_User/statuses/afaba698-5740-4e32-a702-af61aa543bc1", boost.BoostOfURI)
}
func (suite *AnnounceTestSuite) TestAnnounceTwice() {
@@ -81,8 +83,10 @@ func (suite *AnnounceTestSuite) TestAnnounceTwice() {
return nil
})
- // only the URI will be set on the boosted status because it still needs to be dereferenced
- suite.NotEmpty(boost.BoostOf.URI)
+ // only the URI will be set for the boosted status
+ // because it still needs to be dereferenced
+ suite.Nil(boost.BoostOf)
+ suite.Equal("http://example.org/users/Some_User/statuses/afaba698-5740-4e32-a702-af61aa543bc1", boost.BoostOfURI)
ctx2 := createTestContext(receivingAccount2, announcingAccount)
announce2 := suite.testActivities["announce_forwarded_1_turtle"]
diff --git a/internal/gtsmodel/status.go b/internal/gtsmodel/status.go
index 9b93e34a1..79c67c933 100644
--- a/internal/gtsmodel/status.go
+++ b/internal/gtsmodel/status.go
@@ -50,6 +50,7 @@ type Status struct {
InReplyTo *Status `bun:"-"` // status corresponding to inReplyToID
InReplyToAccount *Account `bun:"rel:belongs-to"` // account corresponding to inReplyToAccountID
BoostOfID string `bun:"type:CHAR(26),nullzero"` // id of the status this status is a boost of
+ BoostOfURI string `bun:"-"` // URI of the status this status is a boost of; field not inserted in the db, just for dereferencing purposes.
BoostOfAccountID string `bun:"type:CHAR(26),nullzero"` // id of the account that owns the boosted status
BoostOf *Status `bun:"-"` // status that corresponds to boostOfID
BoostOfAccount *Account `bun:"rel:belongs-to"` // account that corresponds to boostOfAccountID
diff --git a/internal/processing/common/status.go b/internal/processing/common/status.go
index 233c1c867..0a1f495fb 100644
--- a/internal/processing/common/status.go
+++ b/internal/processing/common/status.go
@@ -119,6 +119,25 @@ func (p *Processor) GetVisibleTargetStatus(
return target, nil
}
+// UnwrapIfBoost "unwraps" the given status if
+// it's a boost wrapper, by returning the boosted
+// status it targets (pending visibility checks).
+//
+// Just returns the input status if it's not a boost.
+func (p *Processor) UnwrapIfBoost(
+ ctx context.Context,
+ requester *gtsmodel.Account,
+ status *gtsmodel.Status,
+) (*gtsmodel.Status, gtserror.WithCode) {
+ if status.BoostOfID == "" {
+ return status, nil
+ }
+
+ return p.GetVisibleTargetStatus(ctx,
+ requester, status.BoostOfID,
+ )
+}
+
// GetAPIStatus fetches the appropriate API status model for target.
func (p *Processor) GetAPIStatus(
ctx context.Context,
diff --git a/internal/processing/fedi/status.go b/internal/processing/fedi/status.go
index 8082c79bc..1c1af9cb4 100644
--- a/internal/processing/fedi/status.go
+++ b/internal/processing/fedi/status.go
@@ -51,6 +51,11 @@ func (p *Processor) StatusGet(ctx context.Context, requestedUser string, statusI
return nil, gtserror.NewErrorNotFound(errors.New(text))
}
+ if status.BoostOfID != "" {
+ const text = "status is a boost wrapper"
+ return nil, gtserror.NewErrorNotFound(errors.New(text))
+ }
+
visible, err := p.filter.StatusVisible(ctx, requester, status)
if err != nil {
return nil, gtserror.NewErrorInternalError(err)
@@ -106,6 +111,11 @@ func (p *Processor) StatusRepliesGet(
return nil, gtserror.NewErrorNotFound(errors.New(text))
}
+ if status.BoostOfID != "" {
+ const text = "status is a boost wrapper"
+ return nil, gtserror.NewErrorNotFound(errors.New(text))
+ }
+
// Parse replies collection ID from status' URI with onlyOtherAccounts param.
onlyOtherAccStr := "only_other_accounts=" + strconv.FormatBool(onlyOtherAccounts)
collectionID, err := url.Parse(status.URI + "/replies?" + onlyOtherAccStr)
diff --git a/internal/processing/status/boost.go b/internal/processing/status/boost.go
index 76a0a75bc..2062fb802 100644
--- a/internal/processing/status/boost.go
+++ b/internal/processing/status/boost.go
@@ -30,106 +30,125 @@ import (
"github.com/superseriousbusiness/gotosocial/internal/messages"
)
-// BoostCreate processes the boost/reblog of a given status, returning the newly-created boost if all is well.
-func (p *Processor) BoostCreate(ctx context.Context, requestingAccount *gtsmodel.Account, application *gtsmodel.Application, targetStatusID string) (*apimodel.Status, gtserror.WithCode) {
- targetStatus, err := p.state.DB.GetStatusByID(ctx, targetStatusID)
+// BoostCreate processes the boost/reblog of target
+// status, returning the newly-created boost.
+func (p *Processor) BoostCreate(
+ ctx context.Context,
+ requester *gtsmodel.Account,
+ application *gtsmodel.Application,
+ targetID string,
+) (*apimodel.Status, gtserror.WithCode) {
+ // Get target status and ensure it's not a boost.
+ target, errWithCode := p.c.GetVisibleTargetStatus(
+ ctx,
+ requester,
+ targetID,
+ )
+ if errWithCode != nil {
+ return nil, errWithCode
+ }
+
+ target, errWithCode = p.c.UnwrapIfBoost(
+ ctx,
+ requester,
+ target,
+ )
+ if errWithCode != nil {
+ return nil, errWithCode
+ }
+
+ // Ensure valid boost target.
+ boostable, err := p.filter.StatusBoostable(ctx,
+ requester,
+ target,
+ )
if err != nil {
- return nil, gtserror.NewErrorNotFound(fmt.Errorf("error fetching status %s: %s", targetStatusID, err))
- }
- if targetStatus.Account == nil {
- return nil, gtserror.NewErrorNotFound(fmt.Errorf("no status owner for status %s", targetStatusID))
- }
-
- // if targetStatusID refers to a boost, then we should redirect
- // the target to being the status that was boosted; if we don't
- // do this, then we end up in weird situations where people
- // boost boosts, and it looks absolutely bizarre in the UI
- if targetStatus.BoostOfID != "" {
- if targetStatus.BoostOf == nil {
- b, err := p.state.DB.GetStatusByID(ctx, targetStatus.BoostOfID)
- if err != nil {
- return nil, gtserror.NewErrorNotFound(fmt.Errorf("couldn't fetch boosted status %s", targetStatus.BoostOfID))
- }
- targetStatus.BoostOf = b
- }
- targetStatus = targetStatus.BoostOf
+ err := gtserror.Newf("error seeing if status %s is boostable: %w", target.ID, err)
+ return nil, gtserror.NewErrorInternalError(err)
}
- boostable, err := p.filter.StatusBoostable(ctx, requestingAccount, targetStatus)
- if err != nil {
- return nil, gtserror.NewErrorNotFound(fmt.Errorf("error seeing if status %s is boostable: %s", targetStatus.ID, err))
- } else if !boostable {
- return nil, gtserror.NewErrorNotFound(errors.New("status is not boostable"))
+ if !boostable {
+ err := gtserror.New("status is not boostable")
+ return nil, gtserror.NewErrorNotFound(err)
}
- // it's visible! it's boostable! so let's boost the FUCK out of it
- boostWrapperStatus, err := p.converter.StatusToBoost(ctx, targetStatus, requestingAccount)
+ // Status is visible and boostable.
+ boost, err := p.converter.StatusToBoost(ctx,
+ target,
+ requester,
+ application.ID,
+ )
if err != nil {
return nil, gtserror.NewErrorInternalError(err)
}
- boostWrapperStatus.CreatedWithApplicationID = application.ID
- boostWrapperStatus.BoostOfAccount = targetStatus.Account
-
- // put the boost in the database
- if err := p.state.DB.PutStatus(ctx, boostWrapperStatus); err != nil {
+ // Store the new boost.
+ if err := p.state.DB.PutStatus(ctx, boost); err != nil {
return nil, gtserror.NewErrorInternalError(err)
}
- // send it back to the processor for async processing
+ // Process side effects asynchronously.
p.state.Workers.EnqueueClientAPI(ctx, messages.FromClientAPI{
APObjectType: ap.ActivityAnnounce,
APActivityType: ap.ActivityCreate,
- GTSModel: boostWrapperStatus,
- OriginAccount: requestingAccount,
- TargetAccount: targetStatus.Account,
+ GTSModel: boost,
+ OriginAccount: requester,
+ TargetAccount: target.Account,
})
- return p.c.GetAPIStatus(ctx, requestingAccount, boostWrapperStatus)
+ return p.c.GetAPIStatus(ctx, requester, boost)
}
-// BoostRemove processes the unboost/unreblog of a given status, returning the status if all is well.
-func (p *Processor) BoostRemove(ctx context.Context, requestingAccount *gtsmodel.Account, application *gtsmodel.Application, targetStatusID string) (*apimodel.Status, gtserror.WithCode) {
- targetStatus, err := p.state.DB.GetStatusByID(ctx, targetStatusID)
- if err != nil {
- return nil, gtserror.NewErrorNotFound(fmt.Errorf("error fetching status %s: %s", targetStatusID, err))
- }
- if targetStatus.Account == nil {
- return nil, gtserror.NewErrorNotFound(fmt.Errorf("no status owner for status %s", targetStatusID))
- }
-
- visible, err := p.filter.StatusVisible(ctx, requestingAccount, targetStatus)
- if err != nil {
- return nil, gtserror.NewErrorNotFound(fmt.Errorf("error seeing if status %s is visible: %s", targetStatus.ID, err))
- }
- if !visible {
- return nil, gtserror.NewErrorNotFound(errors.New("status is not visible"))
- }
-
- // Check whether the requesting account has boosted the given status ID.
- boost, err := p.state.DB.GetStatusBoost(ctx, targetStatusID, requestingAccount.ID)
- if err != nil {
- return nil, gtserror.NewErrorNotFound(fmt.Errorf("error checking status boost %s: %w", targetStatusID, err))
+// BoostRemove processes the unboost/unreblog of
+// target status, returning the target status.
+func (p *Processor) BoostRemove(
+ ctx context.Context,
+ requester *gtsmodel.Account,
+ application *gtsmodel.Application,
+ targetID string,
+) (*apimodel.Status, gtserror.WithCode) {
+ // Get target status and ensure it's not a boost.
+ target, errWithCode := p.c.GetVisibleTargetStatus(
+ ctx,
+ requester,
+ targetID,
+ )
+ if errWithCode != nil {
+ return nil, errWithCode
+ }
+
+ target, errWithCode = p.c.UnwrapIfBoost(
+ ctx,
+ requester,
+ target,
+ )
+ if errWithCode != nil {
+ return nil, errWithCode
+ }
+
+ // Check whether requester has actually
+ // boosted target, by trying to get the boost.
+ boost, err := p.state.DB.GetStatusBoost(ctx,
+ target.ID,
+ requester.ID,
+ )
+ if err != nil && !errors.Is(err, db.ErrNoEntries) {
+ err = gtserror.Newf("db error getting boost of %s: %w", target.ID, err)
+ return nil, gtserror.NewErrorInternalError(err)
}
if boost != nil {
- // pin some stuff onto the boost while we have it out of the db
- boost.Account = requestingAccount
- boost.BoostOf = targetStatus
- boost.BoostOfAccount = targetStatus.Account
- boost.BoostOf.Account = targetStatus.Account
-
- // send it back to the processor for async processing
+ // Status was boosted. Process unboost side effects asynchronously.
p.state.Workers.EnqueueClientAPI(ctx, messages.FromClientAPI{
APObjectType: ap.ActivityAnnounce,
APActivityType: ap.ActivityUndo,
GTSModel: boost,
- OriginAccount: requestingAccount,
- TargetAccount: targetStatus.Account,
+ OriginAccount: requester,
+ TargetAccount: target.Account,
})
}
- return p.c.GetAPIStatus(ctx, requestingAccount, targetStatus)
+ return p.c.GetAPIStatus(ctx, requester, target)
}
// StatusBoostedBy returns a slice of accounts that have boosted the given status, filtered according to privacy settings.
diff --git a/internal/processing/status/fave.go b/internal/processing/status/fave.go
index a16fb6620..dbeba7fe9 100644
--- a/internal/processing/status/fave.go
+++ b/internal/processing/status/fave.go
@@ -33,24 +33,46 @@ import (
"github.com/superseriousbusiness/gotosocial/internal/uris"
)
-func (p *Processor) getFaveableStatus(ctx context.Context, requestingAccount *gtsmodel.Account, targetStatusID string) (*gtsmodel.Status, *gtsmodel.StatusFave, gtserror.WithCode) {
- targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx, requestingAccount, targetStatusID)
+func (p *Processor) getFaveableStatus(
+ ctx context.Context,
+ requester *gtsmodel.Account,
+ targetID string,
+) (
+ *gtsmodel.Status,
+ *gtsmodel.StatusFave,
+ gtserror.WithCode,
+) {
+ // Get target status and ensure it's not a boost.
+ target, errWithCode := p.c.GetVisibleTargetStatus(
+ ctx,
+ requester,
+ targetID,
+ )
+ if errWithCode != nil {
+ return nil, nil, errWithCode
+ }
+
+ target, errWithCode = p.c.UnwrapIfBoost(
+ ctx,
+ requester,
+ target,
+ )
if errWithCode != nil {
return nil, nil, errWithCode
}
- if !*targetStatus.Likeable {
+ if !*target.Likeable {
err := errors.New("status is not faveable")
return nil, nil, gtserror.NewErrorForbidden(err, err.Error())
}
- fave, err := p.state.DB.GetStatusFave(ctx, requestingAccount.ID, targetStatusID)
+ fave, err := p.state.DB.GetStatusFave(ctx, requester.ID, target.ID)
if err != nil && !errors.Is(err, db.ErrNoEntries) {
err = fmt.Errorf("getFaveTarget: error checking existing fave: %w", err)
return nil, nil, gtserror.NewErrorInternalError(err)
}
- return targetStatus, fave, nil
+ return target, fave, nil
}
// FaveCreate adds a fave for the requestingAccount, targeting the given status (no-op if fave already exists).
diff --git a/internal/processing/workers/fromfediapi.go b/internal/processing/workers/fromfediapi.go
index 4c5c85666..d04e4ab8d 100644
--- a/internal/processing/workers/fromfediapi.go
+++ b/internal/processing/workers/fromfediapi.go
@@ -26,7 +26,6 @@ import (
"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
- "github.com/superseriousbusiness/gotosocial/internal/id"
"github.com/superseriousbusiness/gotosocial/internal/log"
"github.com/superseriousbusiness/gotosocial/internal/messages"
"github.com/superseriousbusiness/gotosocial/internal/processing/account"
@@ -332,45 +331,44 @@ func (p *fediAPI) CreateLike(ctx context.Context, fMsg messages.FromFediAPI) err
}
func (p *fediAPI) CreateAnnounce(ctx context.Context, fMsg messages.FromFediAPI) error {
- status, ok := fMsg.GTSModel.(*gtsmodel.Status)
+ boost, ok := fMsg.GTSModel.(*gtsmodel.Status)
if !ok {
return gtserror.Newf("%T not parseable as *gtsmodel.Status", fMsg.GTSModel)
}
// Dereference status that this boosts, note
- // that this will handle dereferencing the status
+ // that this will handle storing the boost in
+ // the db, and dereferencing the target status
// ancestors / descendants where appropriate.
- if err := p.federate.DereferenceAnnounce(ctx,
- status,
+ var err error
+ boost, err = p.federate.EnrichAnnounce(
+ ctx,
+ boost,
fMsg.ReceivingAccount.Username,
- ); err != nil {
- return gtserror.Newf("error dereferencing announce: %w", err)
- }
-
- // Generate an ID for the boost wrapper status.
- statusID, err := id.NewULIDFromTime(status.CreatedAt)
+ )
if err != nil {
- return gtserror.Newf("error generating id: %w", err)
- }
- status.ID = statusID
+ if gtserror.IsUnretrievable(err) {
+ // Boosted status domain blocked, nothing to do.
+ log.Debugf(ctx, "skipping announce: %v", err)
+ return nil
+ }
- // Store the boost wrapper status.
- if err := p.state.DB.PutStatus(ctx, status); err != nil {
- return gtserror.Newf("db error inserting status: %w", err)
+ // Actual error.
+ return gtserror.Newf("error dereferencing announce: %w", err)
}
// Timeline and notify the announce.
- if err := p.surface.timelineAndNotifyStatus(ctx, status); err != nil {
+ if err := p.surface.timelineAndNotifyStatus(ctx, boost); err != nil {
log.Errorf(ctx, "error timelining and notifying status: %v", err)
}
- if err := p.surface.notifyAnnounce(ctx, status); err != nil {
+ if err := p.surface.notifyAnnounce(ctx, boost); err != nil {
log.Errorf(ctx, "error notifying announce: %v", err)
}
// Interaction counts changed on the original status;
// uncache the prepared version from all timelines.
- p.surface.invalidateStatusFromTimelines(ctx, status.ID)
+ p.surface.invalidateStatusFromTimelines(ctx, boost.BoostOfID)
return nil
}
diff --git a/internal/processing/workers/fromfediapi_test.go b/internal/processing/workers/fromfediapi_test.go
index 952c008cc..799eaf2dc 100644
--- a/internal/processing/workers/fromfediapi_test.go
+++ b/internal/processing/workers/fromfediapi_test.go
@@ -45,9 +45,7 @@ func (suite *FromFediAPITestSuite) TestProcessFederationAnnounce() {
boostingAccount := suite.testAccounts["remote_account_1"]
announceStatus := &gtsmodel.Status{}
announceStatus.URI = "https://example.org/some-announce-uri"
- announceStatus.BoostOf = &gtsmodel.Status{
- URI: boostedStatus.URI,
- }
+ announceStatus.BoostOfURI = boostedStatus.URI
announceStatus.CreatedAt = time.Now()
announceStatus.UpdatedAt = time.Now()
announceStatus.AccountID = boostingAccount.ID
diff --git a/internal/typeutils/astointernal.go b/internal/typeutils/astointernal.go
index ad42a687d..5bcb35d75 100644
--- a/internal/typeutils/astointernal.go
+++ b/internal/typeutils/astointernal.go
@@ -527,29 +527,15 @@ func (c *Converter) ASBlockToBlock(ctx context.Context, blockable ap.Blockable)
}, nil
}
-// ASAnnounceToStatus converts an activitystreams 'announce' into a status.
-//
-// The returned bool indicates whether this status is new (true) or not new (false).
-//
-// In other words, if the status is already in the database with the ID set on the announceable, then that will be returned,
-// the returned bool will be false, and no further processing is necessary. If the returned bool is true, indicating
-// that this is a new announce, then further processing will be necessary, because the returned status will be bareboned and
-// require further dereferencing.
-//
-// This is useful when multiple users on an instance might receive the same boost, and we only want to process the boost once.
-//
-// NOTE -- this is different from one status being boosted multiple times! In this case, new boosts should indeed be created.
-//
-// Implementation note: this function creates and returns a boost WRAPPER
-// status which references the boosted status in its BoostOf field. No
-// dereferencing is done on the boosted status by this function. Callers
-// should look at `status.BoostOf` to see the status being boosted, and do
-// dereferencing on it as appropriate.
-//
-// The returned boolean indicates whether or not the boost has already been
-// seen before by this instance. If it was, then status.BoostOf should be a
-// fully filled-out status. If not, then only status.BoostOf.URI will be set.
-func (c *Converter) ASAnnounceToStatus(ctx context.Context, announceable ap.Announceable) (*gtsmodel.Status, bool, error) {
+// ASAnnounceToStatus converts an activitystreams 'announce' into a boost
+// wrapper status. The returned bool indicates whether this boost is new
+// (true) or not. If new, callers should use `status.BoostOfURI` to see the
+// status being boosted, and do dereferencing on it as appropriate. If not
+// new, then the boost has already been fully processed and can be ignored.
+func (c *Converter) ASAnnounceToStatus(
+ ctx context.Context,
+ announceable ap.Announceable,
+) (*gtsmodel.Status, bool, error) {
// Default assume
// we already have.
isNew := false
@@ -565,21 +551,21 @@ func (c *Converter) ASAnnounceToStatus(ctx context.Context, announceable ap.Anno
uri := uriObj.String()
// Check if we already have this boost in the database.
- status, err := c.state.DB.GetStatusByURI(ctx, uri)
+ boost, err := c.state.DB.GetStatusByURI(ctx, uri)
if err != nil && !errors.Is(err, db.ErrNoEntries) {
err = gtserror.Newf("db error trying to get status with uri %s: %w", uri, err)
return nil, isNew, err
}
- if status != nil {
- // We already have this status,
+ if boost != nil {
+ // We already have this boost,
// no need to proceed further.
- return status, isNew, nil
+ return boost, isNew, nil
}
- // Create DB status with URI
- status = new(gtsmodel.Status)
- status.URI = uri
+ // Create boost with URI
+ boost = new(gtsmodel.Status)
+ boost.URI = uri
isNew = true
// Get the URI of the boosted status.
@@ -590,22 +576,21 @@ func (c *Converter) ASAnnounceToStatus(ctx context.Context, announceable ap.Anno
}
// Set the URI of the boosted status on
- // the new status, for later dereferencing.
- status.BoostOf = new(gtsmodel.Status)
- status.BoostOf.URI = boostOf[0].String()
+ // the boost, for later dereferencing.
+ boost.BoostOfURI = boostOf[0].String()
// Extract published time for the boost,
// zero-time will fall back to db defaults.
if pub := ap.GetPublished(announceable); !pub.IsZero() {
- status.CreatedAt = pub
- status.UpdatedAt = pub
+ boost.CreatedAt = pub
+ boost.UpdatedAt = pub
} else {
log.Warnf(ctx, "unusable published property on %s", uri)
}
// Extract and load the boost actor account,
// (this MUST already be in database by now).
- status.Account, err = c.getASActorAccount(ctx,
+ boost.Account, err = c.getASActorAccount(ctx,
uri,
announceable,
)
@@ -614,13 +599,13 @@ func (c *Converter) ASAnnounceToStatus(ctx context.Context, announceable ap.Anno
}
// Set the related status<->account fields.
- status.AccountURI = status.Account.URI
- status.AccountID = status.Account.ID
+ boost.AccountURI = boost.Account.URI
+ boost.AccountID = boost.Account.ID
// Calculate intended visibility of the boost.
- status.Visibility, err = ap.ExtractVisibility(
+ boost.Visibility, err = ap.ExtractVisibility(
announceable,
- status.Account.FollowersURI,
+ boost.Account.FollowersURI,
)
if err != nil {
err := gtserror.Newf("error extracting status visibility for %s: %w", uri, err)
@@ -629,15 +614,15 @@ func (c *Converter) ASAnnounceToStatus(ctx context.Context, announceable ap.Anno
// Below IDs will all be included in the
// boosted status, so set them empty here.
- status.AttachmentIDs = make([]string, 0)
- status.TagIDs = make([]string, 0)
- status.MentionIDs = make([]string, 0)
- status.EmojiIDs = make([]string, 0)
-
- // Remaining fields on the boost status will be taken
- // from the boosted status; it's not our job to do all
- // that dereferencing here.
- return status, isNew, nil
+ boost.AttachmentIDs = make([]string, 0)
+ boost.TagIDs = make([]string, 0)
+ boost.MentionIDs = make([]string, 0)
+ boost.EmojiIDs = make([]string, 0)
+
+ // Remaining fields on the boost will be
+ // taken from the target status; it's not
+ // our job to do all that dereferencing here.
+ return boost, isNew, nil
}
// ASFlagToReport converts a remote activitystreams 'flag' representation into a gts model report.
diff --git a/internal/typeutils/astointernal_test.go b/internal/typeutils/astointernal_test.go
index 40bf4c855..4be2434e5 100644
--- a/internal/typeutils/astointernal_test.go
+++ b/internal/typeutils/astointernal_test.go
@@ -463,6 +463,46 @@ func (suite *ASToInternalTestSuite) TestParseFlag6() {
suite.Equal(report.Comment, "misinformation")
}
+func (suite *ASToInternalTestSuite) TestParseAnnounce() {
+ // Boost a status that belongs to a local account
+ boostingAccount := suite.testAccounts["remote_account_1"]
+ targetStatus := suite.testStatuses["local_account_2_status_1"]
+ receivingAccount := suite.testAccounts["local_account_1"]
+
+ raw := `{
+ "@context": "https://www.w3.org/ns/activitystreams",
+ "actor": "` + boostingAccount.URI + `",
+ "id": "http://fossbros-anonymous.io/db22128d-884e-4358-9935-6a7c3940535d",
+ "object": ["` + targetStatus.URI + `"],
+ "type": "Announce",
+ "to": "` + receivingAccount.URI + `"
+ }`
+
+ t := suite.jsonToType(raw)
+ asAnnounce, ok := t.(ap.Announceable)
+ if !ok {
+ suite.FailNow("type not coercible")
+ }
+
+ boost, isNew, err := suite.typeconverter.ASAnnounceToStatus(context.Background(), asAnnounce)
+ if err != nil {
+ suite.FailNow(err.Error())
+ }
+
+ suite.True(isNew)
+ suite.NotNil(boost)
+ suite.Equal(boostingAccount.ID, boost.AccountID)
+ suite.NotNil(boost.Account)
+
+ // Of the 'BoostOf' fields, only BoostOfURI will be set.
+ // Others are set in dereferencing.EnrichAnnounceSafely.
+ suite.Equal(targetStatus.URI, boost.BoostOfURI)
+ suite.Empty(boost.BoostOfID)
+ suite.Nil(boost.BoostOf)
+ suite.Empty(boost.BoostOfAccountID)
+ suite.Nil(boost.BoostOfAccount)
+}
+
func TestASToInternalTestSuite(t *testing.T) {
suite.Run(t, new(ASToInternalTestSuite))
}
diff --git a/internal/typeutils/internal.go b/internal/typeutils/internal.go
index 1824a4421..095e2b121 100644
--- a/internal/typeutils/internal.go
+++ b/internal/typeutils/internal.go
@@ -19,90 +19,86 @@ package typeutils
import (
"context"
- "time"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
"github.com/superseriousbusiness/gotosocial/internal/id"
"github.com/superseriousbusiness/gotosocial/internal/uris"
+ "github.com/superseriousbusiness/gotosocial/internal/util"
)
-// FollowRequestToFollow just converts a follow request into a follow, that's it! No bells and whistles.
-func (c *Converter) FollowRequestToFollow(ctx context.Context, f *gtsmodel.FollowRequest) *gtsmodel.Follow {
- showReblogs := *f.ShowReblogs
- notify := *f.Notify
+// FollowRequestToFollow just converts a follow request
+// into a follow, that's it! No bells and whistles.
+func (c *Converter) FollowRequestToFollow(
+ ctx context.Context,
+ fr *gtsmodel.FollowRequest,
+) *gtsmodel.Follow {
return &gtsmodel.Follow{
- ID: f.ID,
- CreatedAt: f.CreatedAt,
- UpdatedAt: f.UpdatedAt,
- AccountID: f.AccountID,
- TargetAccountID: f.TargetAccountID,
- ShowReblogs: &showReblogs,
- URI: f.URI,
- Notify: &notify,
+ ID: fr.ID,
+ CreatedAt: fr.CreatedAt,
+ UpdatedAt: fr.UpdatedAt,
+ AccountID: fr.AccountID,
+ TargetAccountID: fr.TargetAccountID,
+ ShowReblogs: util.Ptr(*fr.ShowReblogs),
+ URI: fr.URI,
+ Notify: util.Ptr(*fr.Notify),
}
}
-// StatusToBoost wraps the given status into a boosting status.
-func (c *Converter) StatusToBoost(ctx context.Context, s *gtsmodel.Status, boostingAccount *gtsmodel.Account) (*gtsmodel.Status, error) {
- // the wrapper won't use the same ID as the boosted status so we generate some new UUIDs
- accountURIs := uris.GenerateURIsForAccount(boostingAccount.Username)
- boostWrapperStatusID := id.NewULID()
- boostWrapperStatusURI := accountURIs.StatusesURI + "/" + boostWrapperStatusID
- boostWrapperStatusURL := accountURIs.StatusesURL + "/" + boostWrapperStatusID
+// StatusToBoost wraps the target status into a
+// boost wrapper status owned by the requester.
+func (c *Converter) StatusToBoost(
+ ctx context.Context,
+ target *gtsmodel.Status,
+ booster *gtsmodel.Account,
+ applicationID string,
+) (*gtsmodel.Status, error) {
+ // The boost won't use the same IDs as the
+ // target so we need to generate new ones.
+ boostID := id.NewULID()
+ accountURIs := uris.GenerateURIsForAccount(booster.Username)
- local := true
- if boostingAccount.Domain != "" {
- local = false
- }
-
- sensitive := *s.Sensitive
- federated := *s.Federated
- boostable := *s.Boostable
- replyable := *s.Replyable
- likeable := *s.Likeable
+ boost := &gtsmodel.Status{
+ ID: boostID,
+ URI: accountURIs.StatusesURI + "/" + boostID,
+ URL: accountURIs.StatusesURL + "/" + boostID,
- boostWrapperStatus := &gtsmodel.Status{
- ID: boostWrapperStatusID,
- URI: boostWrapperStatusURI,
- URL: boostWrapperStatusURL,
+ // Inherit some fields from the booster account.
+ Local: util.Ptr(booster.IsLocal()),
+ AccountID: booster.ID,
+ Account: booster,
+ AccountURI: booster.URI,
+ CreatedWithApplicationID: applicationID,
- // the boosted status is not created now, but the boost certainly is
- CreatedAt: time.Now(),
- UpdatedAt: time.Now(),
- Local: &local,
- AccountID: boostingAccount.ID,
- AccountURI: boostingAccount.URI,
-
- // replies can be boosted, but boosts are never replies
+ // Replies can be boosted, but
+ // boosts are never replies.
InReplyToID: "",
InReplyToAccountID: "",
- // these will all be wrapped in the boosted status so set them empty here
+ // These will all be wrapped in the
+ // boosted status so set them empty.
AttachmentIDs: []string{},
TagIDs: []string{},
MentionIDs: []string{},
EmojiIDs: []string{},
- // the below fields will be taken from the target status
- Content: s.Content,
- ContentWarning: s.ContentWarning,
- ActivityStreamsType: s.ActivityStreamsType,
- Sensitive: &sensitive,
- Language: s.Language,
- Text: s.Text,
- BoostOfID: s.ID,
- BoostOfAccountID: s.AccountID,
- Visibility: s.Visibility,
- Federated: &federated,
- Boostable: &boostable,
- Replyable: &replyable,
- Likeable: &likeable,
-
- // attach these here for convenience -- the boosted status/account won't go in the DB
- // but they're needed in the processor and for the frontend. Since we have them, we can
- // attach them so we don't need to fetch them again later (save some DB calls)
- BoostOf: s,
+ // Remaining fields all
+ // taken from boosted status.
+ Content: target.Content,
+ ContentWarning: target.ContentWarning,
+ ActivityStreamsType: target.ActivityStreamsType,
+ Sensitive: util.Ptr(*target.Sensitive),
+ Language: target.Language,
+ Text: target.Text,
+ BoostOfID: target.ID,
+ BoostOf: target,
+ BoostOfAccountID: target.AccountID,
+ BoostOfAccount: target.Account,
+ Visibility: target.Visibility,
+ Federated: util.Ptr(*target.Federated),
+ Boostable: util.Ptr(*target.Boostable),
+ Replyable: util.Ptr(*target.Replyable),
+ Likeable: util.Ptr(*target.Likeable),
}
- return boostWrapperStatus, nil
+ return boost, nil
}
diff --git a/internal/typeutils/internaltoas_test.go b/internal/typeutils/internaltoas_test.go
index 0e0607279..cbeaf3c8c 100644
--- a/internal/typeutils/internaltoas_test.go
+++ b/internal/typeutils/internaltoas_test.go
@@ -724,10 +724,11 @@ func (suite *InternalToASTestSuite) TestSelfBoostFollowersOnlyToAS() {
testStatus := suite.testStatuses["local_account_1_status_5"]
testAccount := suite.testAccounts["local_account_1"]
- boostWrapperStatus, err := suite.typeconverter.StatusToBoost(ctx, testStatus, testAccount)
+ boostWrapperStatus, err := suite.typeconverter.StatusToBoost(ctx, testStatus, testAccount, "")
suite.NoError(err)
suite.NotNil(boostWrapperStatus)
+ // Set some fields to predictable values for the test.
boostWrapperStatus.ID = "01G74JJ1KS331G2JXHRMZCE0ER"
boostWrapperStatus.URI = "http://localhost:8080/users/the_mighty_zork/statuses/01G74JJ1KS331G2JXHRMZCE0ER"
boostWrapperStatus.CreatedAt = testrig.TimeMustParse("2022-06-09T13:12:00Z")
diff --git a/internal/typeutils/internaltofrontend.go b/internal/typeutils/internaltofrontend.go
index cd53d9ae6..fe5c8ec8f 100644
--- a/internal/typeutils/internaltofrontend.go
+++ b/internal/typeutils/internaltofrontend.go
@@ -733,11 +733,18 @@ func (c *Converter) statusToFrontend(
s *gtsmodel.Status,
requestingAccount *gtsmodel.Account,
) (*apimodel.Status, error) {
+ // Try to populate status struct pointer fields.
+ // We can continue in many cases of partial failure,
+ // but there are some fields we actually need.
if err := c.state.DB.PopulateStatus(ctx, s); err != nil {
- // Ensure author account present + correct;
- // can't really go further without this!
if s.Account == nil {
- return nil, gtserror.Newf("error(s) populating status, cannot continue: %w", err)
+ err = gtserror.Newf("error(s) populating status, cannot continue (status.Account not set): %w", err)
+ return nil, err
+ }
+
+ if s.BoostOfID != "" && s.BoostOf == nil {
+ err = gtserror.Newf("error(s) populating status, cannot continue (status.BoostOfID set, but status.Boost not set): %w", err)
+ return nil, err
}
log.Errorf(ctx, "error(s) populating status, will continue: %v", err)
@@ -794,12 +801,12 @@ func (c *Converter) statusToFrontend(
apiStatus := &apimodel.Status{
ID: s.ID,
CreatedAt: util.FormatISO8601(s.CreatedAt),
- InReplyToID: nil,
- InReplyToAccountID: nil,
+ InReplyToID: nil, // Set below.
+ InReplyToAccountID: nil, // Set below.
Sensitive: *s.Sensitive,
SpoilerText: s.ContentWarning,
Visibility: c.VisToAPIVis(ctx, s.Visibility),
- Language: nil,
+ Language: nil, // Set below.
URI: s.URI,
URL: s.URL,
RepliesCount: repliesCount,
@@ -811,56 +818,47 @@ func (c *Converter) statusToFrontend(
Reblogged: interacts.Reblogged,
Pinned: interacts.Pinned,
Content: s.Content,
- Reblog: nil,
- Application: nil,
+ Reblog: nil, // Set below.
+ Application: nil, // Set below.
Account: apiAuthorAccount,
MediaAttachments: apiAttachments,
Mentions: apiMentions,
Tags: apiTags,
Emojis: apiEmojis,
Card: nil, // TODO: implement cards
- Poll: nil, // TODO: implement polls
Text: s.Text,
}
// Nullable fields.
-
if s.InReplyToID != "" {
- apiStatus.InReplyToID = func() *string { i := s.InReplyToID; return &i }()
+ apiStatus.InReplyToID = util.Ptr(s.InReplyToID)
}
if s.InReplyToAccountID != "" {
- apiStatus.InReplyToAccountID = func() *string { i := s.InReplyToAccountID; return &i }()
+ apiStatus.InReplyToAccountID = util.Ptr(s.InReplyToAccountID)
}
if s.Language != "" {
- apiStatus.Language = func() *string { i := s.Language; return &i }()
+ apiStatus.Language = util.Ptr(s.Language)
}
if s.BoostOf != nil {
- apiBoostOf, err := c.StatusToAPIStatus(ctx, s.BoostOf, requestingAccount)
+ reblog, err := c.StatusToAPIStatus(ctx, s.BoostOf, requestingAccount)
if err != nil {
return nil, gtserror.Newf("error converting boosted status: %w", err)
}
- apiStatus.Reblog = &apimodel.StatusReblogged{Status: apiBoostOf}
+ apiStatus.Reblog = &apimodel.StatusReblogged{reblog}
}
- if appID := s.CreatedWithApplicationID; appID != "" {
- app := s.CreatedWithApplication
- if app == nil {
- app, err = c.state.DB.GetApplicationByID(ctx, appID)
- if err != nil {
- return nil, gtserror.Newf("error getting application %s: %w", appID, err)
- }
- }
-
- apiApp, err := c.AppToAPIAppPublic(ctx, app)
+ if app := s.CreatedWithApplication; app != nil {
+ apiStatus.Application, err = c.AppToAPIAppPublic(ctx, app)
if err != nil {
- return nil, gtserror.Newf("error converting application %s: %w", appID, err)
+ return nil, gtserror.Newf(
+ "error converting application %s: %w",
+ s.CreatedWithApplicationID, err,
+ )
}
-
- apiStatus.Application = apiApp
}
if s.Poll != nil {
diff --git a/internal/web/thread.go b/internal/web/thread.go
index 523cf7579..20145cfcd 100644
--- a/internal/web/thread.go
+++ b/internal/web/thread.go
@@ -20,6 +20,7 @@ package web
import (
"context"
"encoding/json"
+ "errors"
"fmt"
"net/http"
"strings"
@@ -124,6 +125,13 @@ func (m *Module) threadGETHandler(c *gin.Context) {
return
}
+ // Don't render boosts/reblogs as top-level statuses.
+ if status.Reblog != nil {
+ err := errors.New("status is a boost wrapper / reblog")
+ apiutil.WebErrorHandler(c, gtserror.NewErrorNotFound(err), instanceGet)
+ return
+ }
+
// Fill in the rest of the thread context.
context, errWithCode := m.processor.Status().WebContextGet(ctx, targetStatusID)
if errWithCode != nil {