Skip to content

Commit

Permalink
Accept any string as a key for m.direct account data
Browse files Browse the repository at this point in the history
  • Loading branch information
MatMaul committed Nov 13, 2024
1 parent aca83fb commit 2022ae9
Show file tree
Hide file tree
Showing 14 changed files with 154 additions and 88 deletions.
117 changes: 64 additions & 53 deletions Cargo.lock

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ once_cell = "1.16.0"
pin-project-lite = "0.2.9"
rand = "0.8.5"
reqwest = { version = "0.12.4", default-features = false }
ruma = { version = "0.11.1", features = [
ruma = { git = "https://github.com/ruma/ruma.git", rev = "35fda7f2f7811156df2e60b223dbf136fc143bc8", features = [
"client-api-c",
"compat-upload-signatures",
"compat-user-id",
Expand All @@ -59,7 +59,7 @@ ruma = { version = "0.11.1", features = [
"unstable-msc4075",
"unstable-msc4140",
] }
ruma-common = "0.14.1"
ruma-common = { git = "https://github.com/ruma/ruma.git", rev = "35fda7f2f7811156df2e60b223dbf136fc143bc8" }
serde = "1.0.151"
serde_html_form = "0.2.0"
serde_json = "1.0.91"
Expand Down
12 changes: 7 additions & 5 deletions crates/matrix-sdk-base/src/response_processors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ use std::{
};

use ruma::{
events::{AnyGlobalAccountDataEvent, GlobalAccountDataEventType},
events::{
direct::OwnedDirectUserIdentifier, AnyGlobalAccountDataEvent, GlobalAccountDataEventType,
},
serde::Raw,
OwnedUserId, RoomId,
RoomId,
};
use tracing::{debug, instrument, trace, warn};

Expand Down Expand Up @@ -94,10 +96,10 @@ impl AccountDataProcessor {
for event in events {
let AnyGlobalAccountDataEvent::Direct(direct_event) = event else { continue };

let mut new_dms = HashMap::<&RoomId, HashSet<OwnedUserId>>::new();
for (user_id, rooms) in direct_event.content.iter() {
let mut new_dms = HashMap::<&RoomId, HashSet<OwnedDirectUserIdentifier>>::new();
for (user_identifier, rooms) in direct_event.content.iter() {
for room_id in rooms {
new_dms.entry(room_id).or_default().insert(user_id.clone());
new_dms.entry(room_id).or_default().insert(user_identifier.clone());
}
}

Expand Down
3 changes: 2 additions & 1 deletion crates/matrix-sdk-base/src/rooms/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use ruma::{
events::{
beacon_info::BeaconInfoEventContent,
call::member::{CallMemberEventContent, CallMemberStateKey},
direct::OwnedDirectUserIdentifier,
macros::EventContent,
room::{
avatar::RoomAvatarEventContent,
Expand Down Expand Up @@ -127,7 +128,7 @@ pub struct BaseRoomInfo {
pub(crate) create: Option<MinimalStateEvent<RoomCreateWithCreatorEventContent>>,
/// A list of user ids this room is considered as direct message, if this
/// room is a DM.
pub(crate) dm_targets: HashSet<OwnedUserId>,
pub(crate) dm_targets: HashSet<OwnedDirectUserIdentifier>,
/// The `m.room.encryption` event content that enabled E2EE in this room.
pub(crate) encryption: Option<RoomEncryptionEventContent>,
/// The guest access policy of this room.
Expand Down
3 changes: 2 additions & 1 deletion crates/matrix-sdk-base/src/rooms/normal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use ruma::{
api::client::sync::sync_events::v3::RoomSummary as RumaSummary,
events::{
call::member::{CallMemberStateKey, MembershipData},
direct::OwnedDirectUserIdentifier,
ignored_user_list::IgnoredUserListEventContent,
receipt::{Receipt, ReceiptThread, ReceiptType},
room::{
Expand Down Expand Up @@ -460,7 +461,7 @@ impl Room {
/// only be considered as guidance. We leave members in this list to allow
/// us to re-find a DM with a user even if they have left, since we may
/// want to re-invite them.
pub fn direct_targets(&self) -> HashSet<OwnedUserId> {
pub fn direct_targets(&self) -> HashSet<OwnedDirectUserIdentifier> {
self.inner.read().base_info.dm_targets.clone()
}

Expand Down
28 changes: 15 additions & 13 deletions crates/matrix-sdk-base/src/sliding_sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ mod tests {
api::client::sync::sync_events::UnreadNotificationsCount,
assign, event_id,
events::{
direct::DirectEventContent,
direct::{DirectEventContent, DirectUserIdentifier, OwnedDirectUserIdentifier},
room::{
avatar::RoomAvatarEventContent,
canonical_alias::RoomCanonicalAliasEventContent,
Expand Down Expand Up @@ -1331,7 +1331,7 @@ mod tests {
create_dm(&client, room_id, user_a_id, user_b_id, MembershipState::Join).await;

// (Sanity: B is a direct target, and is in Join state)
assert!(direct_targets(&client, room_id).contains(user_b_id));
assert!(direct_targets(&client, room_id).contains(<&DirectUserIdentifier>::from(user_b_id)));
assert_eq!(membership(&client, room_id, user_b_id).await, MembershipState::Join);

// When B leaves
Expand All @@ -1340,7 +1340,7 @@ mod tests {
// Then B is still a direct target, and is in Leave state (B is a direct target
// because we want to return to our old DM in the UI even if the other
// user left, so we can reinvite them. See https://github.com/matrix-org/matrix-rust-sdk/issues/2017)
assert!(direct_targets(&client, room_id).contains(user_b_id));
assert!(direct_targets(&client, room_id).contains(<&DirectUserIdentifier>::from(user_b_id)));
assert_eq!(membership(&client, room_id, user_b_id).await, MembershipState::Leave);
}

Expand All @@ -1356,7 +1356,7 @@ mod tests {
create_dm(&client, room_id, user_a_id, user_b_id, MembershipState::Invite).await;

// (Sanity: B is a direct target, and is in Invite state)
assert!(direct_targets(&client, room_id).contains(user_b_id));
assert!(direct_targets(&client, room_id).contains(<&DirectUserIdentifier>::from(user_b_id)));
assert_eq!(membership(&client, room_id, user_b_id).await, MembershipState::Invite);

// When B declines the invitation (i.e. leaves)
Expand All @@ -1365,7 +1365,7 @@ mod tests {
// Then B is still a direct target, and is in Leave state (B is a direct target
// because we want to return to our old DM in the UI even if the other
// user left, so we can reinvite them. See https://github.com/matrix-org/matrix-rust-sdk/issues/2017)
assert!(direct_targets(&client, room_id).contains(user_b_id));
assert!(direct_targets(&client, room_id).contains(<&DirectUserIdentifier>::from(user_b_id)));
assert_eq!(membership(&client, room_id, user_b_id).await, MembershipState::Leave);
}

Expand All @@ -1383,7 +1383,7 @@ mod tests {
assert_eq!(membership(&client, room_id, user_a_id).await, MembershipState::Join);

// (Sanity: B is a direct target, and is in Join state)
assert!(direct_targets(&client, room_id).contains(user_b_id));
assert!(direct_targets(&client, room_id).contains(<&DirectUserIdentifier>::from(user_b_id)));
assert_eq!(membership(&client, room_id, user_b_id).await, MembershipState::Join);

let room = client.get_room(room_id).unwrap();
Expand All @@ -1407,7 +1407,7 @@ mod tests {
assert_eq!(membership(&client, room_id, user_a_id).await, MembershipState::Join);

// (Sanity: B is a direct target, and is in Join state)
assert!(direct_targets(&client, room_id).contains(user_b_id));
assert!(direct_targets(&client, room_id).contains(<&DirectUserIdentifier>::from(user_b_id)));
assert_eq!(membership(&client, room_id, user_b_id).await, MembershipState::Invite);

let room = client.get_room(room_id).unwrap();
Expand Down Expand Up @@ -2552,9 +2552,10 @@ mod tests {
let mut room_response = http::response::Room::new();
set_room_joined(&mut room_response, user_a_id);
let mut response = response_with_room(room_id_1, room_response);
let mut direct_content = BTreeMap::new();
direct_content.insert(user_a_id.to_owned(), vec![room_id_1.to_owned()]);
direct_content.insert(user_b_id.to_owned(), vec![room_id_2.to_owned()]);
let mut direct_content: BTreeMap<OwnedDirectUserIdentifier, Vec<OwnedRoomId>> =
BTreeMap::new();
direct_content.insert(user_a_id.into(), vec![room_id_1.to_owned()]);
direct_content.insert(user_b_id.into(), vec![room_id_2.to_owned()]);
response
.extensions
.account_data
Expand Down Expand Up @@ -2665,7 +2666,7 @@ mod tests {
member.membership().clone()
}

fn direct_targets(client: &BaseClient, room_id: &RoomId) -> HashSet<OwnedUserId> {
fn direct_targets(client: &BaseClient, room_id: &RoomId) -> HashSet<OwnedDirectUserIdentifier> {
let room = client.get_room(room_id).expect("Room not found!");
room.direct_targets()
}
Expand Down Expand Up @@ -2724,8 +2725,9 @@ mod tests {
user_id: OwnedUserId,
room_ids: Vec<OwnedRoomId>,
) {
let mut direct_content = BTreeMap::new();
direct_content.insert(user_id, room_ids);
let mut direct_content: BTreeMap<OwnedDirectUserIdentifier, Vec<OwnedRoomId>> =
BTreeMap::new();
direct_content.insert(user_id.into(), room_ids);
response
.extensions
.account_data
Expand Down
5 changes: 3 additions & 2 deletions crates/matrix-sdk-base/src/store/migration_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use std::{
use matrix_sdk_common::deserialized_responses::SyncTimelineEvent;
use ruma::{
events::{
direct::OwnedDirectUserIdentifier,
room::{
avatar::RoomAvatarEventContent,
canonical_alias::RoomCanonicalAliasEventContent,
Expand All @@ -37,7 +38,7 @@ use ruma::{
},
EmptyStateKey, EventContent, RedactContent, StateEventContent, StateEventType,
},
OwnedRoomId, OwnedUserId, RoomId,
OwnedRoomId, RoomId,
};
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -155,7 +156,7 @@ fn encryption_state_default() -> bool {
struct BaseRoomInfoV1 {
avatar: Option<MinimalStateEvent<RoomAvatarEventContent>>,
canonical_alias: Option<MinimalStateEvent<RoomCanonicalAliasEventContent>>,
dm_targets: HashSet<OwnedUserId>,
dm_targets: HashSet<OwnedDirectUserIdentifier>,
encryption: Option<RoomEncryptionEventContent>,
guest_access: Option<MinimalStateEvent<RoomGuestAccessEventContent>>,
history_visibility: Option<MinimalStateEvent<RoomHistoryVisibilityEventContent>>,
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,7 @@ impl Account {
};

for user_id in user_ids {
content.entry(user_id.to_owned()).or_default().push(room_id.to_owned());
content.entry(user_id.into()).or_default().push(room_id.to_owned());
}

// TODO: We should probably save the fact that we need to send this out
Expand Down
7 changes: 5 additions & 2 deletions crates/matrix-sdk/src/encryption/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ use ruma::{
uiaa::{AuthData, UiaaInfo},
},
assign,
events::room::{MediaSource, ThumbnailInfo},
events::{
direct::DirectUserIdentifier,
room::{MediaSource, ThumbnailInfo},
},
DeviceId, OwnedDeviceId, OwnedUserId, TransactionId, UserId,
};
use serde::Deserialize;
Expand Down Expand Up @@ -607,7 +610,7 @@ impl Client {
// Find the room we share with the `user_id` and only with `user_id`
let room = rooms.into_iter().find(|r| {
let targets = r.direct_targets();
targets.len() == 1 && targets.contains(user_id)
targets.len() == 1 && targets.contains(<&DirectUserIdentifier>::from(user_id))
});

trace!(?room, "Found room");
Expand Down
2 changes: 1 addition & 1 deletion crates/matrix-sdk/src/room/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ impl Room {
room_members.retain(|member| member.user_id() != self.own_user_id());

for member in room_members {
let entry = content.entry(member.user_id().to_owned()).or_default();
let entry = content.entry(member.user_id().into()).or_default();
if !entry.iter().any(|room_id| room_id == this_room_id) {
entry.push(this_room_id.to_owned());
}
Expand Down
9 changes: 7 additions & 2 deletions crates/matrix-sdk/tests/integration/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ use ruma::{
assign, device_id,
directory::Filter,
event_id,
events::{direct::DirectEventContent, AnyInitialStateEvent},
events::{
direct::{DirectEventContent, OwnedDirectUserIdentifier},
AnyInitialStateEvent,
},
room_id,
serde::Raw,
user_id, OwnedUserId,
Expand Down Expand Up @@ -496,7 +499,9 @@ async fn test_marking_room_as_dm() {
"The body of the PUT /account_data request should be a valid DirectEventContent",
);

let bob_entry = content.get(bob).expect("We should have bob in the direct event content");
let bob_entry = content
.get(&OwnedDirectUserIdentifier::from(bob.to_owned()))
.expect("We should have bob in the direct event content");

assert_eq!(content.len(), 2, "We should have entries for bob and foo");
assert_eq!(bob_entry.len(), 3, "Bob should have 3 direct rooms");
Expand Down
3 changes: 2 additions & 1 deletion crates/matrix-sdk/tests/integration/room/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use matrix_sdk_test::{
use ruma::{
event_id,
events::{
direct::DirectUserIdentifier,
room::{
avatar::{self, RoomAvatarEventContent},
member::MembershipState,
Expand Down Expand Up @@ -833,7 +834,7 @@ async fn test_is_direct() {
// The room is direct now.
let direct_targets = room.direct_targets();
assert_eq!(direct_targets.len(), 1);
assert!(direct_targets.contains(*BOB));
assert!(direct_targets.contains(<&DirectUserIdentifier>::from(*BOB)));
assert!(room.is_direct().await.unwrap());

// Unset the room as direct.
Expand Down
35 changes: 34 additions & 1 deletion crates/matrix-sdk/tests/integration/room/joined.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@ use ruma::{
api::client::{membership::Invite3pidInit, receipt::create_receipt::v3::ReceiptType},
assign, event_id,
events::{
direct::DirectUserIdentifier,
receipt::ReceiptThread,
room::message::{RoomMessageEventContent, RoomMessageEventContentWithoutRelation},
TimelineEventType,
},
int, mxc_uri, owned_event_id, room_id, thirdparty, user_id, OwnedUserId, TransactionId,
};
use serde_json::{json, Value};
use serde_json::{from_value, json, Value};
use wiremock::{
matchers::{body_json, body_partial_json, header, method, path_regex},
Mock, ResponseTemplate,
Expand Down Expand Up @@ -630,6 +631,38 @@ async fn test_reset_power_levels() {
room.reset_power_levels().await.unwrap();
}

#[async_test]
async fn test_is_direct_invite_by_3pid() {
let (client, server) = logged_in_client_with_server().await;

let mut sync_builder = SyncResponseBuilder::new();
sync_builder.add_joined_room(JoinedRoomBuilder::default());
let data = json!({
"content": {
"[email protected]": [*DEFAULT_TEST_ROOM_ID],
},
"event_id": "$757957878228ekrDs:localhost",
"origin_server_ts": 17195787,
"sender": "@example:localhost",
"state_key": "",
"type": "m.direct",
"unsigned": {
"age": 139298
}
});
sync_builder.add_global_account_data_bulk(vec![from_value(data).unwrap()]);

mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
mock_encryption_state(&server, false).await;

let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000));
let _response = client.sync_once(sync_settings).await.unwrap();

let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap();
assert!(room.is_direct().await.unwrap());
assert!(room.direct_targets().contains(<&DirectUserIdentifier>::from("[email protected]")));
}

#[async_test]
async fn test_call_notifications_ring_for_dms() {
let (client, server) = logged_in_client_with_server().await;
Expand Down
12 changes: 9 additions & 3 deletions crates/matrix-sdk/tests/integration/room/left.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ use matrix_sdk_test::{
async_test, test_json, GlobalAccountDataTestEvent, LeftRoomBuilder, SyncResponseBuilder,
DEFAULT_TEST_ROOM_ID,
};
use ruma::{events::direct::DirectEventContent, user_id, OwnedRoomOrAliasId};
use ruma::{
events::direct::{DirectEventContent, DirectUserIdentifier, OwnedDirectUserIdentifier},
user_id, OwnedRoomOrAliasId,
};
use serde_json::json;
use wiremock::{
matchers::{header, method, path, path_regex},
Expand Down Expand Up @@ -70,7 +73,7 @@ async fn test_forget_direct_room() {
let room = client.get_room(&DEFAULT_TEST_ROOM_ID).unwrap();
assert_eq!(room.state(), RoomState::Left);
assert!(room.is_direct().await.unwrap());
assert!(room.direct_targets().contains(invited_user_id));
assert!(room.direct_targets().contains(<&DirectUserIdentifier>::from(invited_user_id)));

let direct_account_data = client
.account()
Expand All @@ -80,7 +83,10 @@ async fn test_forget_direct_room() {
.expect("no m.direct account data")
.deserialize()
.expect("failed to deserialize m.direct account data");
assert_matches!(direct_account_data.get(invited_user_id), Some(invited_user_dms));
assert_matches!(
direct_account_data.get(&OwnedDirectUserIdentifier::from(invited_user_id)),
Some(invited_user_dms)
);
assert_eq!(invited_user_dms, &[DEFAULT_TEST_ROOM_ID.to_owned()]);

Mock::given(method("POST"))
Expand Down

0 comments on commit 2022ae9

Please sign in to comment.