[bugfix] parent status replied to status not dereferenced sometimes (#2587)

* much simplified DereferenceStatusAncestors(), also handles edge cases now

* perform status acceptibility check before handling even as forward

* don't further dereference ancestors if they're up to date

* call enrichStatusSafely() directly to ensure we get error messages

* change getStatusByURI() semantics to return error + old model on failed update, fix deref ancestor to check for staleness before refetch

* perform a nil-check on the status.Local variable, in case it hasn't been set on new status attempting refresh

* more consistently set returned parent status, don't check if updated

* only home-timeline statuses if explicitly visible AND not explicitly invisible!

* fix broken test now that status acceptibility checks happen on forwarded statuses

2 files changed, 44 insertions, 28 deletions

diff --git a/internal/federation/federatingdb/create.go b/internal/federation/federatingdb/create.go
index 3a8d8f0ac..1008e5f7f 100644
--- a/internal/federation/federatingdb/create.go
+++ b/internal/federation/federatingdb/create.go
@@ -303,27 +303,9 @@ func (f *federatingDB) createStatusable(
 	statusable ap.Statusable,
 	forwarded bool,
 ) error {
-	// If we do have a forward, we should ignore the content
-	// and instead deref based on the URI of the statusable.
-	//
-	// In other words, don't automatically trust whoever sent
-	// this status to us, but fetch the authentic article from
-	// the server it originated from.
-	if forwarded {
-		// Pass the statusable URI (APIri) into the processor
-		// worker and do the rest of the processing asynchronously.
-		f.state.Workers.EnqueueFediAPI(ctx, messages.FromFediAPI{
-			APObjectType:     ap.ObjectNote,
-			APActivityType:   ap.ActivityCreate,
-			APIri:            ap.GetJSONLDId(statusable),
-			APObjectModel:    nil,
-			GTSModel:         nil,
-			ReceivingAccount: receiver,
-		})
-		return nil
-	}
 
-	// Check whether we should accept this new status.
+	// Check whether we should accept this new status,
+	// we do this BEFORE even handling forwards to us.
 	accept, err := f.shouldAcceptStatusable(ctx,
 		receiver,
 		requester,
@@ -343,6 +325,27 @@ func (f *federatingDB) createStatusable(
 		return nil
 	}
 
+	// If we do have a forward, we should ignore the content
+	// and instead deref based on the URI of the statusable.
+	//
+	// In other words, don't automatically trust whoever sent
+	// this status to us, but fetch the authentic article from
+	// the server it originated from.
+	if forwarded {
+
+		// Pass the statusable URI (APIri) into the processor
+		// worker and do the rest of the processing asynchronously.
+		f.state.Workers.EnqueueFediAPI(ctx, messages.FromFediAPI{
+			APObjectType:     ap.ObjectNote,
+			APActivityType:   ap.ActivityCreate,
+			APIri:            ap.GetJSONLDId(statusable),
+			APObjectModel:    nil,
+			GTSModel:         nil,
+			ReceivingAccount: receiver,
+		})
+		return nil
+	}
+
 	// Do the rest of the processing asynchronously. The processor
 	// will handle inserting/updating + further dereferencing the status.
 	f.state.Workers.EnqueueFediAPI(ctx, messages.FromFediAPI{
@@ -371,13 +374,11 @@ func (f *federatingDB) shouldAcceptStatusable(ctx context.Context, receiver *gts
 		name := mention.NameString
 
 		switch {
-		case accURI != "":
-			if accURI == receiver.URI ||
-				accURI == receiver.URL {
-				// Mention target is receiver,
-				// they are mentioned in status.
-				return true, nil
-			}
+		case accURI != "" &&
+			accURI == receiver.URI || accURI == receiver.URL:
+			// Mention target is receiver,
+			// they are mentioned in status.
+			return true, nil
 
 		case accURI == "" && name != "":
 			// Only a name was provided, extract the user@domain parts.
diff --git a/internal/federation/federatingdb/create_test.go b/internal/federation/federatingdb/create_test.go
index a1f1a7e18..5f80812bf 100644
--- a/internal/federation/federatingdb/create_test.go
+++ b/internal/federation/federatingdb/create_test.go
@@ -26,6 +26,8 @@ import (
 	"github.com/superseriousbusiness/activity/streams"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
+	"github.com/superseriousbusiness/gotosocial/internal/id"
+	"github.com/superseriousbusiness/gotosocial/internal/util"
 )
 
 type CreateTestSuite struct {
@@ -60,7 +62,20 @@ func (suite *CreateTestSuite) TestCreateNoteForward() {
 
 	create := suite.testActivities["forwarded_message"].Activity
 
-	err := suite.federatingDB.Create(ctx, create)
+	// ensure a follow exists between requesting
+	// and receiving account, this ensures the forward
+	// will be seen as "relevant" and not get dropped.
+	err := suite.db.PutFollow(ctx, &gtsmodel.Follow{
+		ID:              id.NewULID(),
+		URI:             "https://this.is.a.url",
+		AccountID:       receivingAccount.ID,
+		TargetAccountID: requestingAccount.ID,
+		ShowReblogs:     util.Ptr(true),
+		Notify:          util.Ptr(false),
+	})
+	suite.NoError(err)
+
+	err = suite.federatingDB.Create(ctx, create)
 	suite.NoError(err)
 
 	// should be a message heading to the processor now, which we can intercept here