feat(sdk,base,ui,ffi): Continue to remove dead code in SlidingSyncRoom

[Hywan]

This patch continues the clean up of matrix_sdk::sliding_sync::SlidingSyncRoom. Please read commit-by-commit. 3 methods are removed, because either they are never used, or because the data they contain are a duplication with the matrix_sdk_base::Room.

---

• Address [Meta] SlidingSyncRoom tech debt #3079

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (61a5f1b) 83.72% compared to head (a802b73) 83.73%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3084   +/-   ##
=======================================
  Coverage   83.72%   83.73%           
=======================================
  Files         224      224           
  Lines       23508    23497   -11     
=======================================
- Hits        19683    19675    -8     
+ Misses       3825     3822    -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[poljar]

Are we certain that this is not needed? The proxy calculates the room name for us even before we have all the members or events, i.e. it does what display_name() does for us.

I see that we're storing whatever the proxy sends to us as the m.room.name event in the RoomInfo since #2032.

But that PR seems to be questionable at best. Though the PR should mean that these two method do contain the same info.

[bnjbvr]

Oops, auto-merge strikes again 😨

[Hywan]

The SS room name is synced with the room and room info during the sync process. So yeah, this is not needed to keep it in SS room anymore.

[Hywan]

And to reply concretely to your mesasge, normally display_name returns early if RoomInfo contains a name,

matrix-rust-sdk/crates/matrix-sdk-base/src/rooms/normal.rs

Lines 568 to 575 in 40e0066

	async fn calculate_name(&self) -> StoreResult<DisplayName> {
	let summary = {
	let inner = self.inner.read();
	if let Some(name) = inner.name() {
	let name = name.trim();
	return Ok(DisplayName::Named(name.to_owned()));
	}

.

Also, we have this test that ensures the behaviour at the sliding sync level,

matrix-rust-sdk/crates/matrix-sdk-base/src/sliding_sync.rs

Lines 1212 to 1232 in 40e0066

	async fn display_name_from_sliding_sync_overrides_alias() {
	// Given a logged-in client
	let client = logged_in_client().await;
	let room_id = room_id!("!r:e.uk");
	let user_id = user_id!("@u:e.uk");
	let room_alias_id = room_alias_id!("#myroom:e.uk");
	// When the sliding sync response contains an explicit room name as well as an
	// alias
	let mut room = room_with_canonical_alias(room_alias_id, user_id);
	room.name = Some("This came from the server".to_owned());
	let response = response_with_room(room_id, room).await;
	client.process_sliding_sync(&response, &()).await.expect("Failed to process sync");
	// Then the room's name is just exactly what the server supplied
	let client_room = client.get_room(room_id).expect("No room found");
	assert_eq!(
	client_room.display_name().await.unwrap().to_string(),
	"This came from the server"
	);
	}

.

[poljar]

And to reply concretely to your mesasge, normally display_name returns early if RoomInfo contains a name,

I know what display_name() does, it follows the spec: https://spec.matrix.org/unstable/client-server-api/#calculating-the-display-name-for-a-room.

What I'm complaining about is that we put something that isn't a m.room.name event content into the field that always should contain a m.room.name event content.

The thing that the SS proxy returns is not a m.room.name event, it's the result of the calculation I linked previously done by the proxy.

Incidentally this thing works, because we created a fake m.room.name event and put it in the same field, the spec tells you to take this field verbatim. The problem is that this data, while also being a string, is semantically very different.

This might now contain the field "Tom, Dick, and Harry" for a three person room, but this string should be localized and we lost any way of doing and detecting this case because we pretended that it's actually an m.room.name event that contains "Tom, Dick, and Harry".

I should probably post this complaint on the PR instead or just open a new issue. I find such hacks that make things quickly work, but mangle with the correctness and make future features harder to implement, for example like the localization of room display names, the bane of our existence.