[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

1 files changed, 28 insertions, 27 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.