[chore] refactor extractFromCtx a bit (#2646)

internal/federation/federatingdb/accept.go

@@ -41,11 +41,14 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA
 		l.Debug("entering Accept")
 	}
 
-	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
+	activityContext := getActivityContext(ctx)
-	if internal {
+	if activityContext.internal {
 		return nil // Already processed.
 	}
 
+	requestingAcct := activityContext.requestingAcct
+	receivingAcct := activityContext.receivingAcct
+
 	// Iterate all provided objects in the activity.
 	for _, object := range ap.ExtractObjects(accept) {
 
@@ -65,13 +68,13 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA
 
 				// Make sure the creator of the original follow
 				// is the same as whatever inbox this landed in.
-				if gtsFollow.AccountID != receivingAccount.ID {
+				if gtsFollow.AccountID != receivingAcct.ID {
 					return errors.New("ACCEPT: follow account and inbox account were not the same")
 				}
 
 				// Make sure the target of the original follow
 				// is the same as the account making the request.
-				if gtsFollow.TargetAccountID != requestingAccount.ID {
+				if gtsFollow.TargetAccountID != requestingAcct.ID {
 					return errors.New("ACCEPT: follow target account and requesting account were not the same")
 				}
 
@@ -84,7 +87,7 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA
 					APObjectType:     ap.ActivityFollow,
 					APActivityType:   ap.ActivityAccept,
 					GTSModel:         follow,
-					ReceivingAccount: receivingAccount,
+					ReceivingAccount: receivingAcct,
 				})
 			}
 
@@ -112,13 +115,13 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA
 
 			// Make sure the creator of the original follow
 			// is the same as whatever inbox this landed in.
-			if followReq.AccountID != receivingAccount.ID {
+			if followReq.AccountID != receivingAcct.ID {
 				return errors.New("ACCEPT: follow account and inbox account were not the same")
 			}
 
 			// Make sure the target of the original follow
 			// is the same as the account making the request.
-			if followReq.TargetAccountID != requestingAccount.ID {
+			if followReq.TargetAccountID != requestingAcct.ID {
 				return errors.New("ACCEPT: follow target account and requesting account were not the same")
 			}
 
@@ -131,7 +134,7 @@ func (f *federatingDB) Accept(ctx context.Context, accept vocab.ActivityStreamsA
 				APObjectType:     ap.ActivityFollow,
 				APActivityType:   ap.ActivityAccept,
 				GTSModel:         follow,
-				ReceivingAccount: receivingAccount,
+				ReceivingAccount: receivingAcct,
 			})
 
 			continue

internal/federation/federatingdb/announce.go

@@ -41,22 +41,25 @@ func (f *federatingDB) Announce(ctx context.Context, announce vocab.ActivityStre
 		l.Debug("entering Announce")
 	}
 
-	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
+	activityContext := getActivityContext(ctx)
-	if internal {
+	if activityContext.internal {
 		return nil // Already processed.
 	}
 
+	requestingAcct := activityContext.requestingAcct
+	receivingAcct := activityContext.receivingAcct
+
 	// Ensure requestingAccount is among
 	// the Actors doing the Announce.
 	//
 	// We don't support Announce forwards.
 	actorIRIs := ap.GetActorIRIs(announce)
 	if !slices.ContainsFunc(actorIRIs, func(actorIRI *url.URL) bool {
-		return actorIRI.String() == requestingAccount.URI
+		return actorIRI.String() == requestingAcct.URI
 	}) {
 		return gtserror.Newf(
 			"requestingAccount %s was not among Announce Actors",
-			requestingAccount.URI,
+			requestingAcct.URI,
 		)
 	}
 
@@ -76,7 +79,7 @@ func (f *federatingDB) Announce(ctx context.Context, announce vocab.ActivityStre
 		APObjectType:     ap.ActivityAnnounce,
 		APActivityType:   ap.ActivityCreate,
 		GTSModel:         boost,
-		ReceivingAccount: receivingAccount,
+		ReceivingAccount: receivingAcct,
 	})
 
 	return nil

internal/federation/federatingdb/create.go

@@ -63,27 +63,30 @@ func (f *federatingDB) Create(ctx context.Context, asType vocab.Type) error {
 			Trace("entering Create")
 	}
 
-	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
+	activityContext := getActivityContext(ctx)
-	if internal {
+	if activityContext.internal {
 		return nil // Already processed.
 	}
 
+	requestingAcct := activityContext.requestingAcct
+	receivingAcct := activityContext.receivingAcct
+
 	switch asType.GetTypeName() {
 	case ap.ActivityBlock:
 		// BLOCK SOMETHING
-		return f.activityBlock(ctx, asType, receivingAccount, requestingAccount)
+		return f.activityBlock(ctx, asType, receivingAcct, requestingAcct)
 	case ap.ActivityCreate:
 		// CREATE SOMETHING
-		return f.activityCreate(ctx, asType, receivingAccount, requestingAccount)
+		return f.activityCreate(ctx, asType, receivingAcct, requestingAcct)
 	case ap.ActivityFollow:
 		// FOLLOW SOMETHING
-		return f.activityFollow(ctx, asType, receivingAccount, requestingAccount)
+		return f.activityFollow(ctx, asType, receivingAcct, requestingAcct)
 	case ap.ActivityLike:
 		// LIKE SOMETHING
-		return f.activityLike(ctx, asType, receivingAccount, requestingAccount)
+		return f.activityLike(ctx, asType, receivingAcct, requestingAcct)
 	case ap.ActivityFlag:
 		// FLAG / REPORT SOMETHING
-		return f.activityFlag(ctx, asType, receivingAccount, requestingAccount)
+		return f.activityFlag(ctx, asType, receivingAcct, requestingAcct)
 	}
 
 	return nil

internal/federation/federatingdb/delete.go

@@ -40,30 +40,33 @@ func (f *federatingDB) Delete(ctx context.Context, id *url.URL) error {
 		}...)
 	l.Debug("entering Delete")
 
-	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
+	activityContext := getActivityContext(ctx)
-	if internal {
+	if activityContext.internal {
 		return nil // Already processed.
 	}
 
+	requestingAcct := activityContext.requestingAcct
+	receivingAcct := activityContext.receivingAcct
+
 	// in a delete we only get the URI, we can't know if we have a status or a profile or something else,
 	// so we have to try a few different things...
-	if s, err := f.state.DB.GetStatusByURI(ctx, id.String()); err == nil && requestingAccount.ID == s.AccountID {
+	if s, err := f.state.DB.GetStatusByURI(ctx, id.String()); err == nil && requestingAcct.ID == s.AccountID {
 		l.Debugf("uri is for STATUS with id: %s", s.ID)
 		f.state.Workers.EnqueueFediAPI(ctx, messages.FromFediAPI{
 			APObjectType:     ap.ObjectNote,
 			APActivityType:   ap.ActivityDelete,
 			GTSModel:         s,
-			ReceivingAccount: receivingAccount,
+			ReceivingAccount: receivingAcct,
 		})
 	}
 
-	if a, err := f.state.DB.GetAccountByURI(ctx, id.String()); err == nil && requestingAccount.ID == a.ID {
+	if a, err := f.state.DB.GetAccountByURI(ctx, id.String()); err == nil && requestingAcct.ID == a.ID {
 		l.Debugf("uri is for ACCOUNT with id %s", a.ID)
 		f.state.Workers.EnqueueFediAPI(ctx, messages.FromFediAPI{
 			APObjectType:     ap.ObjectProfile,
 			APActivityType:   ap.ActivityDelete,
 			GTSModel:         a,
-			ReceivingAccount: receivingAccount,
+			ReceivingAccount: receivingAcct,
 		})
 	}
 

internal/federation/federatingdb/reject.go

@@ -40,11 +40,14 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR
 		l.Debug("entering Reject")
 	}
 
-	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
+	activityContext := getActivityContext(ctx)
-	if internal {
+	if activityContext.internal {
 		return nil // Already processed.
 	}
 
+	requestingAcct := activityContext.requestingAcct
+	receivingAcct := activityContext.receivingAcct
+
 	for _, obj := range ap.ExtractObjects(reject) {
 
 		if obj.IsIRI() {
@@ -59,13 +62,13 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR
 
 				// Make sure the creator of the original follow
 				// is the same as whatever inbox this landed in.
-				if followReq.AccountID != receivingAccount.ID {
+				if followReq.AccountID != receivingAcct.ID {
 					return errors.New("Reject: follow account and inbox account were not the same")
 				}
 
 				// Make sure the target of the original follow
 				// is the same as the account making the request.
-				if followReq.TargetAccountID != requestingAccount.ID {
+				if followReq.TargetAccountID != requestingAcct.ID {
 					return errors.New("Reject: follow target account and requesting account were not the same")
 				}
 
@@ -89,13 +92,13 @@ func (f *federatingDB) Reject(ctx context.Context, reject vocab.ActivityStreamsR
 
 			// Make sure the creator of the original follow
 			// is the same as whatever inbox this landed in.
-			if gtsFollow.AccountID != receivingAccount.ID {
+			if gtsFollow.AccountID != receivingAcct.ID {
 				return errors.New("Reject: follow account and inbox account were not the same")
 			}
 
 			// Make sure the target of the original follow
 			// is the same as the account making the request.
-			if gtsFollow.TargetAccountID != requestingAccount.ID {
+			if gtsFollow.TargetAccountID != requestingAcct.ID {
 				return errors.New("Reject: follow target account and requesting account were not the same")
 			}
 

internal/federation/federatingdb/undo.go

@@ -44,13 +44,15 @@ func (f *federatingDB) Undo(ctx context.Context, undo vocab.ActivityStreamsUndo)
 		l.Debug("entering Undo")
 	}
 
-	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
+	activityContext := getActivityContext(ctx)
-	if internal {
+	if activityContext.internal {
 		return nil // Already processed.
 	}
 
-	var errs gtserror.MultiError
+	requestingAcct := activityContext.requestingAcct
+	receivingAcct := activityContext.receivingAcct
 
+	var errs gtserror.MultiError
 	for _, object := range ap.ExtractObjects(undo) {
 		// Try to get object as vocab.Type,
 		// else skip handling (likely) IRI.
@@ -61,18 +63,18 @@ func (f *federatingDB) Undo(ctx context.Context, undo vocab.ActivityStreamsUndo)
 
 		switch objType.GetTypeName() {
 		case ap.ActivityFollow:
-			if err := f.undoFollow(ctx, receivingAccount, requestingAccount, undo, objType); err != nil {
+			if err := f.undoFollow(ctx, receivingAcct, requestingAcct, undo, objType); err != nil {
 				errs.Appendf("error undoing follow: %w", err)
 			}
 		case ap.ActivityLike:
-			if err := f.undoLike(ctx, receivingAccount, requestingAccount, undo, objType); err != nil {
+			if err := f.undoLike(ctx, receivingAcct, requestingAcct, undo, objType); err != nil {
 				errs.Appendf("error undoing like: %w", err)
 			}
 		case ap.ActivityAnnounce:
 			// TODO: actually handle this !
 			log.Warn(ctx, "skipped undo announce")
 		case ap.ActivityBlock:
-			if err := f.undoBlock(ctx, receivingAccount, requestingAccount, undo, objType); err != nil {
+			if err := f.undoBlock(ctx, receivingAcct, requestingAcct, undo, objType); err != nil {
 				errs.Appendf("error undoing block: %w", err)
 			}
 		}

internal/federation/federatingdb/update.go

@@ -53,17 +53,20 @@ func (f *federatingDB) Update(ctx context.Context, asType vocab.Type) error {
 		l.Debug("entering Update")
 	}
 
-	receivingAccount, requestingAccount, internal := extractFromCtx(ctx)
+	activityContext := getActivityContext(ctx)
-	if internal {
+	if activityContext.internal {
 		return nil // Already processed.
 	}
 
+	requestingAcct := activityContext.requestingAcct
+	receivingAcct := activityContext.receivingAcct
+
 	if accountable, ok := ap.ToAccountable(asType); ok {
-		return f.updateAccountable(ctx, receivingAccount, requestingAccount, accountable)
+		return f.updateAccountable(ctx, receivingAcct, requestingAcct, accountable)
 	}
 
 	if statusable, ok := ap.ToStatusable(asType); ok {
-		return f.updateStatusable(ctx, receivingAccount, requestingAccount, statusable)
+		return f.updateStatusable(ctx, receivingAcct, requestingAcct, statusable)
 	}
 
 	return nil

internal/federation/federatingdb/util.go

@@ -220,25 +220,52 @@ func (f *federatingDB) collectIRIs(ctx context.Context, iris []*url.URL) (vocab.
 	return collection, nil
 }
 
-// extractFromCtx extracts some useful values from a context passed into the federatingDB:
+// activityContext represents the context in
-//
+// which a call to one of the federatingdb
-//   - The account that owns the inbox or URI being interacted with.
+// functions is taking place, including the
-//   - The account that POSTed a request to the inbox.
+// account who initiated the request via POST
-//   - Whether this is an internal request (one originating not from
+// to an inbox, and the account who received
-//     the API but from inside the instance).
+// the request in their inbox.
-//
+type activityContext struct {
-// If the request is internal, the caller can assume that the activity has
+	// The account that owns the inbox
-// already been processed elsewhere, and should return with no further action.
+	// or URI being interacted with.
-func extractFromCtx(ctx context.Context) (receivingAccount *gtsmodel.Account, requestingAccount *gtsmodel.Account, internal bool) {
+	receivingAcct *gtsmodel.Account
-	receivingAccount = gtscontext.ReceivingAccount(ctx)
-	requestingAccount = gtscontext.RequestingAccount(ctx)
 
-	// If the receiving account wasn't set on the context, that
+	// The account whose keyId was used
-	// means this request didn't pass through the API, but
+	// to POST a request to the inbox.
-	// came from inside GtS as the result of a local activity.
+	requestingAcct *gtsmodel.Account
-	internal = receivingAccount == nil
 
-	return
+	// Whether this is an internal request,
+	// ie., one originating not from the
+	// API but from inside the instance.
+	//
+	// If the request is internal, it's
+	// safe to assume that the activity
+	// has already been processed elsewhere,
+	// and we can return with no action.
+	internal bool
+}
+
+// getActivityContext extracts the context in
+// which an Activity is taking place from the
+// context.Context passed in to one of the
+// federatingdb functions.
+func getActivityContext(ctx context.Context) activityContext {
+	receivingAcct := gtscontext.ReceivingAccount(ctx)
+	requestingAcct := gtscontext.RequestingAccount(ctx)
+
+	// If the receiving account wasn't set on
+	// the context, that means this request
+	// didn't pass through the fedi API, but
+	// came from inside the instance as the
+	// result of a local activity.
+	internal := receivingAcct == nil
+
+	return activityContext{
+		receivingAcct:  receivingAcct,
+		requestingAcct: requestingAcct,
+		internal:       internal,
+	}
 }
 
 func marshalItem(item vocab.Type) (string, error) {