refactor: Consolidate hierarchy and summary logic in a new service

src/api/client/room/mod.rs

@@ -6,10 +6,7 @@ mod summary;
 mod upgrade;
 
 pub(crate) use self::{
-	aliases::get_room_aliases_route,
-	create::create_room_route,
-	event::get_room_event_route,
-	initial_sync::room_initial_sync_route,
-	summary::{get_room_summary, get_room_summary_legacy},
+	aliases::get_room_aliases_route, create::create_room_route, event::get_room_event_route,
+	initial_sync::room_initial_sync_route, summary::get_room_summary,
 	upgrade::upgrade_room_route,
 };

src/api/client/room/summary.rs

@@ -1,48 +1,11 @@
 use axum::extract::State;
 use axum_client_ip::ClientIp;
-use conduwuit::{
-	Err, Result, debug, debug_warn, info, trace,
-	utils::{IterStream, future::TryExtExt},
-};
-use futures::{
-	FutureExt, StreamExt, TryFutureExt,
-	future::{OptionFuture, join3},
-	stream::FuturesUnordered,
-};
-use ruma::{
-	OwnedServerName, RoomId, UserId,
-	api::{
-		client::room::get_summary,
-		federation::space::{SpaceHierarchyParentSummary, get_hierarchy},
-	},
-	events::room::member::MembershipState,
-	space::SpaceRoomJoinRule::{self, *},
-};
-use service::Services;
+use conduwuit::{Err, Result};
+use ruma::api::client::room::get_summary;
+use service::rooms::summary::Accessibility;
 
-use crate::{Ruma, RumaResponse};
+use crate::Ruma;
 
-/// # `GET /_matrix/client/unstable/im.nheko.summary/rooms/{roomIdOrAlias}/summary`
-///
-/// Returns a short description of the state of a room.
-///
-/// This is the "wrong" endpoint that some implementations/clients may use
-/// according to the MSC. Request and response bodies are the same as
-/// `get_room_summary`.
-///
-/// An implementation of [MSC3266](https://github.com/matrix-org/matrix-spec-proposals/pull/3266)
-pub(crate) async fn get_room_summary_legacy(
-	State(services): State<crate::State>,
-	ClientIp(client): ClientIp,
-	body: Ruma<get_summary::msc3266::Request>,
-) -> Result<RumaResponse<get_summary::msc3266::Response>> {
-	get_room_summary(State(services), ClientIp(client), body)
-		.boxed()
-		.await
-		.map(RumaResponse)
-}
-
-/// # `GET /_matrix/client/unstable/im.nheko.summary/summary/{roomIdOrAlias}`
 /// # `GET /_matrix/client/v1/room_summary/{roomIdOrAlias}`
 ///
 /// Returns a short description of the state of a room.
@@ -50,8 +13,8 @@ pub(crate) async fn get_room_summary_legacy(
 pub(crate) async fn get_room_summary(
 	State(services): State<crate::State>,
 	ClientIp(client): ClientIp,
-	body: Ruma<get_summary::msc3266::Request>,
-) -> Result<get_summary::msc3266::Response> {
+	body: Ruma<get_summary::v1::Request>,
+) -> Result<get_summary::v1::Response> {
 	let (room_id, servers) = services
 		.rooms
 		.alias
@@ -62,285 +25,19 @@ pub(crate) async fn get_room_summary(
 		return Err!(Request(Forbidden("This room is banned on this homeserver.")));
 	}
 
-	room_summary_response(&services, &room_id, &servers, body.sender_user.as_deref())
-		.boxed()
-		.await
-}
-
-async fn room_summary_response(
-	services: &Services,
-	room_id: &RoomId,
-	servers: &[OwnedServerName],
-	sender_user: Option<&UserId>,
-) -> Result<get_summary::msc3266::Response> {
-	if services
+	let summary = services
 		.rooms
-		.state_cache
-		.server_in_room(services.globals.server_name(), room_id)
-		.await
-	{
-		match local_room_summary_response(services, room_id, sender_user)
-			.boxed()
-			.await
-		{
-			| Ok(response) => return Ok(response),
-			| Err(e) => {
-				debug_warn!("Failed to get local room summary: {e:?}, falling back to remote");
-			},
-		}
-	}
-
-	let room =
-		remote_room_summary_hierarchy_response(services, room_id, servers, sender_user).await?;
-
-	Ok(get_summary::msc3266::Response {
-		room_id: room_id.to_owned(),
-		canonical_alias: room.canonical_alias,
-		avatar_url: room.avatar_url,
-		guest_can_join: room.guest_can_join,
-		name: room.name,
-		num_joined_members: room.num_joined_members,
-		topic: room.topic,
-		world_readable: room.world_readable,
-		join_rule: room.join_rule,
-		room_type: room.room_type,
-		room_version: room.room_version,
-		encryption: room.encryption,
-		allowed_room_ids: room.allowed_room_ids,
-		membership: sender_user.is_some().then_some(MembershipState::Leave),
-	})
-}
-
-async fn local_room_summary_response(
-	services: &Services,
-	room_id: &RoomId,
-	sender_user: Option<&UserId>,
-) -> Result<get_summary::msc3266::Response> {
-	trace!(
-		sender_user = sender_user.map(tracing::field::display),
-		"Sending local room summary response for {room_id:?}"
-	);
-	let (join_rule, world_readable, guest_can_join) = join3(
-		services.rooms.state_accessor.get_join_rules(room_id),
-		services.rooms.state_accessor.is_world_readable(room_id),
-		services.rooms.state_accessor.guest_can_join(room_id),
-	)
-	.await;
-
-	// Synapse allows server admins to bypass visibility checks.
-	// That seems neat so we'll copy that behaviour.
-	if sender_user.is_none() || !services.users.is_admin(sender_user.unwrap()).await {
-		user_can_see_summary(
-			services,
-			room_id,
-			&join_rule.clone().into(),
-			guest_can_join,
-			world_readable,
-			join_rule.allowed_rooms(),
-			sender_user,
-		)
+		.summary
+		.get_room_summary_for_user(body.sender_user.as_deref(), &room_id, &servers)
 		.await?;
-	}
 
-	let canonical_alias = services
-		.rooms
-		.state_accessor
-		.get_canonical_alias(room_id)
-		.ok();
-
-	let name = services.rooms.state_accessor.get_name(room_id).ok();
-
-	let topic = services.rooms.state_accessor.get_room_topic(room_id).ok();
-
-	let room_type = services.rooms.state_accessor.get_room_type(room_id).ok();
-
-	let avatar_url = services
-		.rooms
-		.state_accessor
-		.get_avatar(room_id)
-		.map(|res| res.into_option().unwrap_or_default().url);
-
-	let room_version = services.rooms.state.get_room_version(room_id).ok();
-
-	let encryption = services
-		.rooms
-		.state_accessor
-		.get_room_encryption(room_id)
-		.ok();
-
-	let num_joined_members = services
-		.rooms
-		.state_cache
-		.room_joined_count(room_id)
-		.unwrap_or(0);
-
-	let membership: OptionFuture<_> = sender_user
-		.map(|sender_user| {
-			services
-				.rooms
-				.state_accessor
-				.get_member(room_id, sender_user)
-				.map_ok_or(MembershipState::Leave, |content| content.membership)
-		})
-		.into();
-
-	let (
-		canonical_alias,
-		name,
-		num_joined_members,
-		topic,
-		avatar_url,
-		room_type,
-		room_version,
-		encryption,
-		membership,
-	) = futures::join!(
-		canonical_alias,
-		name,
-		num_joined_members,
-		topic,
-		avatar_url,
-		room_type,
-		room_version,
-		encryption,
-		membership,
-	);
-
-	Ok(get_summary::msc3266::Response {
-		room_id: room_id.to_owned(),
-		canonical_alias,
-		avatar_url,
-		guest_can_join,
-		name,
-		num_joined_members: num_joined_members.try_into().unwrap_or_default(),
-		topic,
-		world_readable,
-		room_type,
-		room_version,
-		encryption,
-		membership,
-		allowed_room_ids: join_rule.allowed_rooms().map(Into::into).collect(),
-		join_rule: join_rule.into(),
-	})
-}
-
-/// used by MSC3266 to fetch a room's info if we do not know about it
-async fn remote_room_summary_hierarchy_response(
-	services: &Services,
-	room_id: &RoomId,
-	servers: &[OwnedServerName],
-	sender_user: Option<&UserId>,
-) -> Result<SpaceHierarchyParentSummary> {
-	trace!(sender_user = ?sender_user.map(tracing::field::display), ?servers, "Sending remote room summary response for {room_id:?}");
-	if !services.config.allow_federation {
-		return Err!(Request(Forbidden("Federation is disabled.")));
-	}
-
-	if services.rooms.metadata.is_disabled(room_id).await {
-		return Err!(Request(Forbidden(
-			"Federaton of room {room_id} is currently disabled on this server."
-		)));
-	}
-	if servers.is_empty() {
-		return Err!(Request(MissingParam(
-			"No servers were provided to fetch the room over federation"
-		)));
-	}
-
-	let request = get_hierarchy::v1::Request::new(room_id.to_owned());
-
-	let mut requests: FuturesUnordered<_> = servers
-		.iter()
-		.map(|server| {
-			info!("Fetching room summary for {room_id} from server {server}");
-			services
-				.sending
-				.send_federation_request(server, request.clone())
-				.inspect_ok(move |v| {
-					debug!("Fetched room summary for {room_id} from server {server}: {v:?}");
-				})
-				.inspect_err(move |e| {
-					info!("Failed to fetch room summary for {room_id} from server {server}: {e}");
-				})
-		})
-		.collect();
-
-	while let Some(Ok(response)) = requests.next().await {
-		trace!("{response:?}");
-		let room = response.room.clone();
-		if room.room_id != room_id {
-			debug_warn!(
-				"Room ID {} returned does not belong to the requested room ID {}",
-				room.room_id,
-				room_id
-			);
-			continue;
-		}
-
-		if sender_user.is_none() || !services.users.is_admin(sender_user.unwrap()).await {
-			return user_can_see_summary(
-				services,
-				room_id,
-				&room.join_rule,
-				room.guest_can_join,
-				room.world_readable,
-				room.allowed_room_ids.iter().map(AsRef::as_ref),
-				sender_user,
-			)
-			.await
-			.map(|()| room);
-		}
-		return Ok(room);
-	}
-
-	Err!(Request(NotFound("Room not found or is not accessible")))
-}
-
-async fn user_can_see_summary<'a, I>(
-	services: &Services,
-	room_id: &RoomId,
-	join_rule: &SpaceRoomJoinRule,
-	guest_can_join: bool,
-	world_readable: bool,
-	allowed_room_ids: I,
-	sender_user: Option<&UserId>,
-) -> Result
-where
-	I: Iterator<Item = &'a RoomId> + Send,
-{
-	let is_public_room = matches!(join_rule, Public | Knock | KnockRestricted);
-	match sender_user {
-		| Some(sender_user) => {
-			let user_can_see_state_events = services
-				.rooms
-				.state_accessor
-				.user_can_see_state_events(sender_user, room_id);
-			let is_guest = services.users.is_deactivated(sender_user).unwrap_or(false);
-			let user_in_allowed_restricted_room = allowed_room_ids
-				.stream()
-				.any(|room| services.rooms.state_cache.is_joined(sender_user, room));
-
-			let (user_can_see_state_events, is_guest, user_in_allowed_restricted_room) =
-				join3(user_can_see_state_events, is_guest, user_in_allowed_restricted_room)
-					.boxed()
-					.await;
-
-			if user_can_see_state_events
-				|| (is_guest && guest_can_join)
-				|| is_public_room
-				|| user_in_allowed_restricted_room
-			{
-				return Ok(());
-			}
-
-			Err!(Request(Forbidden("Room is not accessible")))
+	match summary {
+		| Accessibility::Accessible(summary) => Ok(get_summary::v1::Response::new(summary)),
+		| Accessibility::Inaccessible => {
+			Err!(Request(Forbidden("You may not preview this room."), FORBIDDEN))
 		},
-		| None => {
-			if is_public_room || world_readable {
-				return Ok(());
-			}
-
-			Err!(Request(Forbidden("Room is not accessible")))
+		| Accessibility::NotFound => {
+			Err!(Request(Forbidden("This room does not exist."), FORBIDDEN))
 		},
 	}
 }

src/api/client/space.rs

@@ -1,26 +1,12 @@
-use std::{
-	collections::{BTreeSet, VecDeque},
-	str::FromStr,
-};
-
 use axum::extract::State;
-use conduwuit::{
-	Err, Result,
-	utils::{future::TryExtExt, stream::IterStream},
-};
-use conduwuit_service::{
-	Services,
-	rooms::spaces::{
-		PaginationToken, SummaryAccessibility, get_parent_children_via, summary_to_chunk,
-	},
-};
-use futures::{StreamExt, TryFutureExt, future::OptionFuture};
-use ruma::{
-	OwnedRoomId, OwnedServerName, RoomId, UInt, UserId, api::client::space::get_hierarchy,
-};
+use conduwuit::{Err, Result};
+use ruma::{UInt, api::client::space::get_hierarchy, assign};
+use service::rooms::summary::Accessibility;
 
 use crate::Ruma;
 
+const MAX_MAX_DEPTH: u32 = 10;
+
 /// # `GET /_matrix/client/v1/rooms/{room_id}/hierarchy`
 ///
 /// Paginates over the space tree in a depth-first manner to locate child rooms
@@ -29,167 +15,33 @@ pub(crate) async fn get_hierarchy_route(
 	State(services): State<crate::State>,
 	body: Ruma<get_hierarchy::v1::Request>,
 ) -> Result<get_hierarchy::v1::Response> {
-	let limit = body
-		.limit
-		.unwrap_or_else(|| UInt::from(10_u32))
-		.min(UInt::from(100_u32));
+	// We don't do pagination for this route (and therefore ignore `limit`), since
+	// there's no reasonable way to handle a space hierarchy changing during
+	// pagination.
 
 	let max_depth = body
 		.max_depth
-		.unwrap_or_else(|| UInt::from(3_u32))
-		.min(UInt::from(10_u32));
+		.map(|max_depth| max_depth.min(UInt::from(MAX_MAX_DEPTH)));
 
-	let key = body
-		.from
-		.as_ref()
-		.and_then(|s| PaginationToken::from_str(s).ok());
+	let hierarchy = services
+		.rooms
+		.summary
+		.get_room_hierarchy_for_user(
+			body.sender_user(),
+			body.room_id.clone(),
+			max_depth,
+			body.suggested_only,
+		)
+		.await?;
 
-	// Should prevent unexpected behaviour in (bad) clients
-	if let Some(ref token) = key {
-		if token.suggested_only != body.suggested_only || token.max_depth != max_depth {
-			return Err!(Request(InvalidParam(
-				"suggested_only and max_depth cannot change on paginated requests"
-			)));
-		}
+	match hierarchy {
+		| Accessibility::Accessible(rooms) =>
+			Ok(assign!(get_hierarchy::v1::Response::new(), { rooms: rooms })),
+		| Accessibility::Inaccessible => {
+			Err!(Request(Forbidden("You may not preview this room."), FORBIDDEN))
+		},
+		| Accessibility::NotFound => {
+			Err!(Request(Forbidden("This room does not exist."), FORBIDDEN))
+		},
 	}
-
-	get_client_hierarchy(
-		&services,
-		body.sender_user(),
-		&body.room_id,
-		limit.try_into().unwrap_or(10),
-		max_depth.try_into().unwrap_or(usize::MAX),
-		body.suggested_only,
-		key.as_ref()
-			.into_iter()
-			.flat_map(|t| t.short_room_ids.iter()),
-	)
-	.await
-}
-
-async fn get_client_hierarchy<'a, ShortRoomIds>(
-	services: &Services,
-	sender_user: &UserId,
-	room_id: &RoomId,
-	limit: usize,
-	max_depth: usize,
-	suggested_only: bool,
-	short_room_ids: ShortRoomIds,
-) -> Result<get_hierarchy::v1::Response>
-where
-	ShortRoomIds: Iterator<Item = &'a u64> + Clone + Send + Sync + 'a,
-{
-	type Via = Vec<OwnedServerName>;
-	type Entry = (OwnedRoomId, Via);
-	type Rooms = VecDeque<Entry>;
-
-	let mut queue: Rooms = [(
-		room_id.to_owned(),
-		room_id
-			.server_name()
-			.map(ToOwned::to_owned)
-			.into_iter()
-			.collect(),
-	)]
-	.into();
-
-	let mut rooms = Vec::with_capacity(limit);
-	let mut parents = BTreeSet::new();
-	while let Some((current_room, via)) = queue.pop_front() {
-		let summary = services
-			.rooms
-			.spaces
-			.get_summary_and_children_client(&current_room, suggested_only, sender_user, &via)
-			.await?;
-
-		match (summary, current_room == room_id) {
-			| (None | Some(SummaryAccessibility::Inaccessible), false) => {
-				// Just ignore other unavailable rooms
-			},
-			| (None, true) => {
-				return Err!(Request(Forbidden("The requested room was not found")));
-			},
-			| (Some(SummaryAccessibility::Inaccessible), true) => {
-				return Err!(Request(Forbidden("The requested room is inaccessible")));
-			},
-			| (Some(SummaryAccessibility::Accessible(summary)), _) => {
-				let populate = parents.len() >= short_room_ids.clone().count();
-
-				let mut children: Vec<Entry> = get_parent_children_via(&summary, suggested_only)
-					.filter(|(room, _)| !parents.contains(room))
-					.rev()
-					.map(|(key, val)| (key, val.collect()))
-					.collect();
-
-				if populate {
-					rooms.push(summary_to_chunk(summary.clone()));
-				} else {
-					children = children
-						.iter()
-						.rev()
-						.stream()
-						.skip_while(|(room, _)| {
-							services
-								.rooms
-								.short
-								.get_shortroomid(room)
-								.map_ok(|short| {
-									Some(&short) != short_room_ids.clone().nth(parents.len())
-								})
-								.unwrap_or_else(|_| false)
-						})
-						.map(Clone::clone)
-						.collect::<Vec<Entry>>()
-						.await
-						.into_iter()
-						.rev()
-						.collect();
-				}
-
-				if !populate && queue.is_empty() && children.is_empty() {
-					break;
-				}
-
-				parents.insert(current_room.clone());
-				if rooms.len() >= limit {
-					break;
-				}
-
-				if parents.len() > max_depth {
-					continue;
-				}
-
-				queue.extend(children);
-			},
-		}
-	}
-
-	let next_batch: OptionFuture<_> = queue
-		.pop_front()
-		.map(|(room, _)| async move {
-			parents.insert(room);
-
-			let next_short_room_ids: Vec<_> = parents
-				.iter()
-				.stream()
-				.filter_map(|room_id| services.rooms.short.get_shortroomid(room_id).ok())
-				.collect()
-				.await;
-
-			(next_short_room_ids.iter().ne(short_room_ids) && !next_short_room_ids.is_empty())
-				.then_some(PaginationToken {
-					short_room_ids: next_short_room_ids,
-					limit: limit.try_into().ok()?,
-					max_depth: max_depth.try_into().ok()?,
-					suggested_only,
-				})
-				.as_ref()
-				.map(PaginationToken::to_string)
-		})
-		.into();
-
-	Ok(get_hierarchy::v1::Response {
-		next_batch: next_batch.await.flatten(),
-		rooms,
-	})
 }

src/api/router.rs

@@ -184,18 +184,14 @@ pub fn build(router: Router<State>, server: &Server) -> Router<State> {
 		.ruma_route(&client::get_hierarchy_route)
 		.ruma_route(&client::get_mutual_rooms_route)
 		.ruma_route(&client::get_room_summary)
-		.route(
-			"/_matrix/client/unstable/im.nheko.summary/rooms/{room_id_or_alias}/summary",
-			get(client::get_room_summary_legacy)
-		)
 		.ruma_route(&client::get_suspended_status)
 		.ruma_route(&client::put_suspended_status)
 		.ruma_route(&client::well_known_support)
 		.ruma_route(&client::well_known_client)
 		.ruma_route(&client::get_rtc_transports)
+		.ruma_route(&client::room_initial_sync_route)
 		.route("/_conduwuit/server_version", get(client::conduwuit_server_version))
 		.route("/_continuwuity/server_version", get(client::conduwuit_server_version))
-		.ruma_route(&client::room_initial_sync_route)
 		.route("/client/server.json", get(client::syncv3_client_server_json))
 		.ruma_route(&admin::rooms::ban::ban_room)
 		.ruma_route(&admin::rooms::list::list_rooms);

src/api/server/hierarchy.rs

@@ -1,13 +1,7 @@
 use axum::extract::State;
-use conduwuit::{
-	Err, Result, info,
-	utils::stream::{BroadbandExt, IterStream},
-};
-use conduwuit_service::rooms::spaces::{
-	Identifier, SummaryAccessibility, get_parent_children_via,
-};
-use futures::{FutureExt, StreamExt};
+use conduwuit::{Err, Result, info};
 use ruma::api::federation::space::get_hierarchy;
+use service::rooms::summary::Accessibility;
 
 use crate::Ruma;
 
@@ -19,10 +13,6 @@ pub(crate) async fn get_hierarchy_route(
 	State(services): State<crate::State>,
 	body: Ruma<get_hierarchy::v1::Request>,
 ) -> Result<get_hierarchy::v1::Response> {
-	if !services.rooms.metadata.exists(&body.room_id).await {
-		return Err!(Request(NotFound("Room does not exist.")));
-	}
-
 	if !services
 		.rooms
 		.state_cache
@@ -36,52 +26,15 @@ pub(crate) async fn get_hierarchy_route(
 		return Err!(Request(NotFound("This server is not participating in that room.")));
 	}
 
-	let room_id = &body.room_id;
-	let suggested_only = body.suggested_only;
-	let ref identifier = Identifier::ServerName(body.origin());
-	match services
+	let response = services
 		.rooms
-		.spaces
-		.get_summary_and_children_local(room_id, identifier)
-		.await?
-	{
-		| None => Err!(Request(NotFound("The requested room was not found"))),
+		.summary
+		.get_local_room_summary_for_server(body.origin(), &body.room_id, body.suggested_only)
+		.await;
 
-		| Some(SummaryAccessibility::Inaccessible) => {
-			Err!(Request(NotFound("The requested room is inaccessible")))
-		},
-
-		| Some(SummaryAccessibility::Accessible(room)) => {
-			let (children, inaccessible_children) =
-				get_parent_children_via(&room, suggested_only)
-					.stream()
-					.broad_filter_map(|(child, _via)| async move {
-						match services
-							.rooms
-							.spaces
-							.get_summary_and_children_local(&child, identifier)
-							.await
-							.ok()?
-						{
-							| None => None,
-
-							| Some(SummaryAccessibility::Inaccessible) =>
-								Some((None, Some(child))),
-
-							| Some(SummaryAccessibility::Accessible(summary)) =>
-								Some((Some(summary), None)),
-						}
-					})
-					.unzip()
-					.map(|(children, inaccessible_children): (Vec<_>, Vec<_>)| {
-						(
-							children.into_iter().flatten().map(Into::into).collect(),
-							inaccessible_children.into_iter().flatten().collect(),
-						)
-					})
-					.await;
-
-			Ok(get_hierarchy::v1::Response { room, children, inaccessible_children })
-		},
+	if let Accessibility::Accessible(response) = response {
+		Ok(response)
+	} else {
+		Err!(Request(NotFound("This room is not accessible.")))
 	}
 }