style: Take an optional context instead of deconstructed parameters

src/api/client/context.rs

@@ -104,10 +104,7 @@ pub(crate) async fn get_context_route(
 				.timeline
 				.get_unsigned_context(&pdu.1, Some(sender_user))
 				.await;
-			pdu.1
-				.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because)
-				.log_err()
-				.ok();
+			pdu.1.set_unsigned(Some(ctx)).log_err().ok();
 			if let Err(e) = services
 				.rooms
 				.pdu_metadata
@@ -135,10 +132,7 @@ pub(crate) async fn get_context_route(
 				.timeline
 				.get_unsigned_context(&pdu.1, Some(sender_user))
 				.await;
-			pdu.1
-				.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because)
-				.log_err()
-				.ok();
+			pdu.1.set_unsigned(Some(ctx)).log_err().ok();
 			if let Err(e) = services
 				.rooms
 				.pdu_metadata

src/api/client/message.rs

@@ -145,10 +145,7 @@ pub(crate) async fn get_message_events_route(
 				.timeline
 				.get_unsigned_context(&pdu.1, Some(sender_user))
 				.await;
-			pdu.1
-				.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because)
-				.log_err()
-				.ok();
+			pdu.1.set_unsigned(Some(ctx)).log_err().ok();
 			if let Err(e) = services
 				.rooms
 				.pdu_metadata

src/api/client/room/event.rs

@@ -63,7 +63,7 @@ pub(crate) async fn get_room_event_route(
 		.timeline
 		.get_unsigned_context(&event, Some(sender_user))
 		.await;
-	event.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because)?;
+	event.set_unsigned(Some(ctx))?;
 
 	Ok(get_room_event::v3::Response::new(event.into_format()))
 }

src/api/client/room/initial_sync.rs

@@ -59,10 +59,7 @@ pub(crate) async fn room_initial_sync_route(
 				.timeline
 				.get_unsigned_context(&pdu.1, Some(sender_user))
 				.await;
-			pdu.1
-				.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because)
-				.log_err()
-				.ok();
+			pdu.1.set_unsigned(Some(ctx)).log_err().ok();
 			if let Err(e) = services
 				.rooms
 				.pdu_metadata

src/api/client/sync/mod.rs

@@ -91,15 +91,7 @@ async fn load_timeline(
 						.timeline
 						.get_unsigned_context(&pdu.1, Some(sender_user))
 						.await;
-					pdu.1
-						.set_unsigned(
-							ctx.user_id,
-							ctx.membership,
-							ctx.prev_content,
-							ctx.redacted_because,
-						)
-						.log_err()
-						.ok();
+					pdu.1.set_unsigned(Some(ctx)).log_err().ok();
 					Some(pdu)
 				})
 				.then(async move |mut pdu| {
@@ -129,15 +121,7 @@ async fn load_timeline(
 						.timeline
 						.get_unsigned_context(&pdu.1, Some(sender_user))
 						.await;
-					pdu.1
-						.set_unsigned(
-							ctx.user_id,
-							ctx.membership,
-							ctx.prev_content,
-							ctx.redacted_because,
-						)
-						.log_err()
-						.ok();
+					pdu.1.set_unsigned(Some(ctx)).log_err().ok();
 					Some(pdu)
 				})
 				.then(async move |mut pdu| {

src/api/client/sync/v3/joined.rs

@@ -328,9 +328,7 @@ async fn build_sticky_events(
 			.timeline
 			.get_unsigned_context(&pdu, Some(sync_context.syncing_user))
 			.await;
-		pdu.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because)
-			.log_err()
-			.ok();
+		pdu.set_unsigned(Some(ctx)).log_err().ok();
 		events.push(Event::into_format(pdu));
 	}
 

src/api/client/sync/v5.rs

@@ -1218,9 +1218,7 @@ async fn push_sticky_event(
 		.timeline
 		.get_unsigned_context(&pdu, Some(sender_user))
 		.await;
-	pdu.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because)
-		.log_err()
-		.ok();
+	pdu.set_unsigned(Some(ctx)).log_err().ok();
 	response
 		.rooms
 		.entry(room_id)

src/core/matrix/pdu.rs

@@ -23,6 +23,7 @@ pub use self::{
 	id::{ShortId, *},
 	partial::PartialPdu,
 	raw_id::*,
+	unsigned::UnsignedContext,
 };
 use super::{Event, StateKey};
 use crate::Result;

src/core/matrix/pdu/tests.rs

@@ -74,7 +74,7 @@ fn redaction_removes_stickiness() {
 fn sticky_ttl_is_added_to_unsigned() {
 	let mut pdu = sticky_pdu(&serde_json::json!({ "duration_ms": 300_000 }));
 	pdu.origin_server_ts = ruma::MilliSecondsSinceUnixEpoch::now().get();
-	pdu.set_unsigned(None, None, None, None).unwrap();
+	pdu.set_unsigned(None).unwrap();
 
 	let unsigned: serde_json::Value =
 		serde_json::from_str(pdu.unsigned.as_deref().expect("unsigned is set").get())
@@ -99,7 +99,7 @@ fn set_unsigned_adds_the_sticky_ttl() {
 	// section
 	let mut pdu = sticky_pdu(&serde_json::json!({ "duration_ms": 300_000 }));
 	pdu.origin_server_ts = ruma::MilliSecondsSinceUnixEpoch::now().get();
-	pdu.set_unsigned(None, None, None, None).unwrap();
+	pdu.set_unsigned(None).unwrap();
 
 	let unsigned: serde_json::Value =
 		serde_json::from_str(pdu.unsigned.as_deref().expect("unsigned is set").get())

src/core/matrix/pdu/unsigned.rs

@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 
 use ruma::{
-	MilliSecondsSinceUnixEpoch,
+	MilliSecondsSinceUnixEpoch, UserId,
 	events::{AnyTimelineEvent, room::member::MembershipState},
 	serde::Raw,
 };
@@ -12,6 +12,14 @@ use crate::Event;
 
 type Unsigned = BTreeMap<String, Box<RawJsonValue>>;
 
+#[derive(Default, Clone)]
+pub struct UnsignedContext<'a> {
+	pub user_id: Option<&'a UserId>,
+	pub membership: Option<MembershipState>,
+	pub prev_content: Option<Box<serde_json::value::RawValue>>,
+	pub redacted_because: Option<Raw<AnyTimelineEvent>>,
+}
+
 impl Pdu {
 	/// Sets the `unsigned` field of the PDU using a mix of the provided
 	/// optional data, and information available in the PDU itself. The
@@ -30,10 +38,7 @@ impl Pdu {
 	/// well-formed.
 	pub fn set_unsigned(
 		&mut self,
-		user_id: Option<&ruma::UserId>,
-		membership: Option<MembershipState>,
-		prev_content: Option<Box<RawJsonValue>>,
-		redacted_because: Option<Raw<AnyTimelineEvent>>,
+		context: Option<UnsignedContext<'_>>,
 	) -> crate::Result<Unsigned> {
 		// See: https://spec.matrix.org/v1.19/client-server-api/#definition-clientevent_unsigneddata
 		let mut unsigned = self
@@ -47,25 +52,25 @@ impl Pdu {
 			to_raw_value(&now_i.saturating_sub(self.origin_server_ts.into()))?,
 		);
 
-		if let Some(membership) = membership {
-			unsigned.insert("membership".to_owned(), to_raw_value(&membership)?);
-		}
+		if let Some(context) = context {
+			if let Some(membership) = context.membership {
+				unsigned.insert("membership".to_owned(), to_raw_value(&membership)?);
+			}
 
-		if let Some(prev_content) = prev_content {
-			unsigned.insert("prev_content".to_owned(), to_raw_value(&prev_content)?);
-			// TODO(nex): prev_sender is still inserted in append.rs because
-			// only the prev *content* is passed to this function.
-			// I think ideally passing the actual direct `UnsignedContext` to
-			// this function instead of unpacking its parameters would be the
-			// proper solution, especially since every callsite has one
-		}
-		if let Some(redacted_because) = redacted_because {
-			unsigned.remove("org.continuwuity.redacted_by");
-			unsigned.insert("redacted_because".to_owned(), to_raw_value(&redacted_because)?);
-		}
+			if let Some(prev_content) = context.prev_content {
+				unsigned.insert("prev_content".to_owned(), to_raw_value(&prev_content)?);
+			}
+			if let Some(redacted_because) = context.redacted_because {
+				unsigned.remove("org.continuwuity.redacted_by");
+				unsigned.insert("redacted_because".to_owned(), to_raw_value(&redacted_because)?);
+			}
 
-		// Remove transaction_id unless the user is the sender
-		if user_id.is_none_or(|u| u != self.sender()) {
+			// replaces_state is embedded and always included.
+
+			if context.user_id.is_none_or(|u| u != self.sender()) {
+				unsigned.remove("transaction_id");
+			}
+		} else {
 			unsigned.remove("transaction_id");
 		}
 

src/service/rooms/pdu_metadata/data.rs

@@ -100,9 +100,7 @@ impl Data {
 				.timeline
 				.get_unsigned_context(&pdu, Some(user_id))
 				.await;
-			pdu.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because)
-				.log_err()
-				.ok();
+			pdu.set_unsigned(Some(ctx)).log_err().ok();
 
 			Some((shorteventid, pdu))
 		})

src/service/rooms/search/mod.rs

@@ -143,14 +143,7 @@ impl Service {
 					.timeline
 					.get_unsigned_context(&pdu, Some(sender_user))
 					.await;
-				pdu.set_unsigned(
-					ctx.user_id,
-					ctx.membership,
-					ctx.prev_content,
-					ctx.redacted_because,
-				)
-				.log_err()
-				.ok();
+				pdu.set_unsigned(Some(ctx)).log_err().ok();
 
 				Some(pdu)
 			})

src/service/rooms/threads/mod.rs

@@ -170,14 +170,7 @@ impl Service {
 					.timeline
 					.get_unsigned_context(&pdu, Some(user_id))
 					.await;
-				pdu.set_unsigned(
-					ctx.user_id,
-					ctx.membership,
-					ctx.prev_content,
-					ctx.redacted_because,
-				)
-				.log_err()
-				.ok();
+				pdu.set_unsigned(Some(ctx)).log_err().ok();
 
 				Some((pdu_id.shorteventid, pdu))
 			});

src/service/rooms/timeline/helpers.rs

@@ -1,8 +1,8 @@
 //! Helpers for submitting events with the right checks performed
 
-use conduwuit::{
+pub(crate) use conduwuit::{
 	Err, Event, Result, err,
-	matrix::pdu::{PartialPdu, sticky},
+	matrix::pdu::{PartialPdu, UnsignedContext, sticky},
 	trace,
 };
 use ruma::{
@@ -23,14 +23,6 @@ use ruma::{
 
 use crate::rooms::state::RoomMutexGuard;
 
-#[derive(Default, Clone)]
-pub struct UnsignedContext<'a> {
-	pub user_id: Option<&'a UserId>,
-	pub membership: Option<MembershipState>,
-	pub prev_content: Option<Box<serde_json::value::RawValue>>,
-	pub redacted_because: Option<Raw<AnyTimelineEvent>>,
-}
-
 impl super::Service {
 	#[allow(clippy::too_many_arguments)]
 	pub async fn send_state_event_for_key(
@@ -369,11 +361,7 @@ impl super::Service {
 					let can_see = self
 						.services
 						.state_accessor
-						.user_can_see_event(
-							sender_user,
-							&event.room_id_or_hash(),
-							event.event_id(),
-						)
+						.user_can_see_event(sender_user, &event.room_id_or_hash(), &event_id)
 						.await;
 					if can_see {
 						trace!(prev_event_id=%event_id, "Fetching prev content");

src/service/rooms/timeline/mod.rs

@@ -27,7 +27,7 @@ use ruma::{
 use serde::Deserialize;
 
 use self::data::Data;
-pub use self::{create::pdu_fits, data::PdusIterItem, helpers::UnsignedContext};
+pub use self::{create::pdu_fits, data::PdusIterItem};
 use crate::{
 	Dep, account_data, admin, appservice, config, globals, pusher, rooms, sending, server_keys,
 	sync, users,