From c556316057b346ad7b6a9d6b1303738a55f67c49 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 8 Jan 2024 15:15:27 +0100 Subject: [PATCH] feat: Remove the need for the `downcast` function. The `downcast` function is removed in this patch because it was incompatible with JS code minifier. Instead, we now rely on the new `wasm_bindgen::convert::TryFromJsValue` API. This patch also replaces `&Array` by `Vec`. It's fine, except that JS arrays that are sent to Rust are going to be dropped after the functions return. It's quite unsual for JS, but documentations have been updated accordingly. --- src/js.rs | 29 -------------------- src/lib.rs | 1 - src/machine.rs | 63 +++++++++++++++++++++---------------------- src/sync_events.rs | 20 +++++--------- tests/machine.test.ts | 9 +++---- 5 files changed, 42 insertions(+), 80 deletions(-) delete mode 100644 src/js.rs diff --git a/src/js.rs b/src/js.rs deleted file mode 100644 index 4bfc9a3c4..000000000 --- a/src/js.rs +++ /dev/null @@ -1,29 +0,0 @@ -use js_sys::{Object, Reflect}; -use wasm_bindgen::{convert::RefFromWasmAbi, prelude::*}; - -/// A really hacky and dirty code to downcast a `JsValue` to `T: -/// RefFromWasmAbi`, inspired by -/// https://github.com/rustwasm/wasm-bindgen/issues/2231#issuecomment-656293288. -/// -/// The returned value is likely to be a `wasm_bindgen::__ref::Ref`. -pub(crate) fn downcast(value: &JsValue, classname: &str) -> Result -where - T: RefFromWasmAbi, -{ - let constructor_name = Object::get_prototype_of(value).constructor().name(); - - if constructor_name == classname { - let pointer = Reflect::get(value, &JsValue::from_str("__wbg_ptr")) - .map_err(|_| JsError::new("Failed to read the `JsValue` pointer"))?; - let pointer = pointer - .as_f64() - .ok_or_else(|| JsError::new("Failed to read the `JsValue` pointer as a `f64`"))? - as u32; - - Ok(unsafe { T::ref_from_abi(pointer) }) - } else { - Err(JsError::new(&format!( - "Expect an `{classname}` instance, received `{constructor_name}` instead", - ))) - } -} diff --git a/src/lib.rs b/src/lib.rs index 649601b0f..b2ccaf5e1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,7 +27,6 @@ pub mod events; mod future; pub mod identifiers; pub mod identities; -mod js; pub mod libolm_migration; pub mod machine; mod macros; diff --git a/src/machine.rs b/src/machine.rs index ee4e780a8..08eb8921d 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -18,7 +18,7 @@ use matrix_sdk_crypto::{ use serde_json::json; use serde_wasm_bindgen; use tracing::warn; -use wasm_bindgen::prelude::*; +use wasm_bindgen::{convert::TryFromJsValue, prelude::*}; use wasm_bindgen_futures::{spawn_local, JsFuture}; use crate::{ @@ -26,9 +26,7 @@ use crate::{ device, encryption, error::MegolmDecryptionError, future::{future_to_promise, future_to_promise_with_custom_error}, - identifiers, identities, - js::downcast, - olm, requests, + identifiers, identities, olm, requests, requests::{outgoing_request_to_js_value, CrossSigningBootstrapRequests, ToDeviceRequest}, responses::{self, response_from_string}, store, @@ -227,16 +225,19 @@ impl OlmMachine { /// "changed" notification for that user in the future. /// /// Users that were already in the list are unaffected. + /// + /// `users` is going to be dropped on the JS part after the function + /// returns. Be careful to not used `users` after that. #[wasm_bindgen(js_name = "updateTrackedUsers")] - pub fn update_tracked_users(&self, users: &Array) -> Result { - let users = user_ids_to_owned_user_ids(users)?; + pub fn update_tracked_users(&self, users: Vec) -> Promise { + let users = users.iter().map(|user| user.inner.clone()).collect::>(); let me = self.inner.clone(); - Ok(future_to_promise(async move { + future_to_promise(async move { me.update_tracked_users(users.iter().map(AsRef::as_ref)).await?; Ok(JsValue::UNDEFINED) - })) + }) } /// Handle to-device events and one-time key counts from a sync @@ -636,21 +637,24 @@ impl OlmMachine { /// time is in flight for the same room, e.g. using a lock. /// /// Returns an array of `ToDeviceRequest`s. + /// + /// `users` is going to be dropped on the JS part after the function + /// returns. Be careful to not used `users` after that. #[wasm_bindgen(js_name = "shareRoomKey")] pub fn share_room_key( &self, room_id: &identifiers::RoomId, - users: &Array, + users: Vec, encryption_settings: &encryption::EncryptionSettings, - ) -> Result { + ) -> Promise { let room_id = room_id.inner.clone(); - let users = user_ids_to_owned_user_ids(users)?; + let users = users.iter().map(|user| user.inner.clone()).collect::>(); let encryption_settings = matrix_sdk_crypto::olm::EncryptionSettings::from(encryption_settings); let me = self.inner.clone(); - Ok(future_to_promise(async move { + future_to_promise(async move { let to_device_requests = me .share_room_key(&room_id, users.iter().map(AsRef::as_ref), encryption_settings) .await?; @@ -664,7 +668,7 @@ impl OlmMachine { .into_iter() .map(|td| ToDeviceRequest::try_from(td.deref()).map(JsValue::from)) .collect::>()?) - })) + }) } /// Generate an "out-of-band" key query request for the given set of users. @@ -674,12 +678,15 @@ impl OlmMachine { /// /// Returns a `KeysQueryRequest` object. The response of the request should /// be passed to the `OlmMachine` with the `mark_request_as_sent`. + /// + /// `users` is going to be dropped on the JS part after the function + /// returns. Be careful to not used `users` after that. #[wasm_bindgen(js_name = "queryKeysForUsers")] pub fn query_keys_for_users( &self, - users: &Array, + users: Vec, ) -> Result { - let users = user_ids_to_owned_user_ids(users)?; + let users = users.iter().map(|user| user.inner.clone()).collect::>(); let (request_id, request) = self.inner.query_keys_for_users(users.iter().map(AsRef::as_ref)); @@ -712,13 +719,16 @@ impl OlmMachine { /// `users` represents the list of users that we should check if /// we lack a session with one of their devices. This can be an /// empty iterator when calling this method between sync requests. + /// + /// `users` is going to be dropped on the JS part after the function + /// returns. Be careful to not used `users` after that. #[wasm_bindgen(js_name = "getMissingSessions")] - pub fn get_missing_sessions(&self, users: &Array) -> Result { - let users = user_ids_to_owned_user_ids(users)?; + pub fn get_missing_sessions(&self, users: Vec) -> Promise { + let users = users.iter().map(|user| user.inner.clone()).collect::>(); let me = self.inner.clone(); - Ok(future_to_promise(async move { + future_to_promise(async move { match me.get_missing_sessions(users.iter().map(AsRef::as_ref)).await? { Some((transaction_id, keys_claim_request)) => { Ok(JsValue::from(requests::KeysClaimRequest::try_from(( @@ -729,7 +739,7 @@ impl OlmMachine { None => Ok(JsValue::NULL), } - })) + }) } /// Get a map holding all the devices of a user. @@ -999,9 +1009,7 @@ impl OlmMachine { let mut keys: BTreeMap<_, BTreeMap<_, _>> = BTreeMap::new(); for backed_up_room_keys_entry in backed_up_room_keys.entries() { let backed_up_room_keys_entry: Array = backed_up_room_keys_entry?.dyn_into()?; - let room_id = - &downcast::(&backed_up_room_keys_entry.get(0), "RoomId")? - .inner; + let room_id = identifiers::RoomId::try_from_js_value(backed_up_room_keys_entry.get(0))?; let room_room_keys: Map = backed_up_room_keys_entry.get(1).dyn_into()?; @@ -1011,7 +1019,7 @@ impl OlmMachine { let key: BackedUpRoomKey = serde_wasm_bindgen::from_value(room_room_keys_entry.get(1))?; - keys.entry(room_id.clone()).or_default().insert(session_id.into(), key); + keys.entry(room_id.inner.clone()).or_default().insert(session_id.into(), key); } } @@ -1457,12 +1465,3 @@ pub(crate) async fn promise_result_to_future( } } } - -/// Helper function to take a Javascript array of `UserId`s and turn it into -/// a Rust `Vec` of `OwnedUserId`s -fn user_ids_to_owned_user_ids(users: &Array) -> Result, JsError> { - users - .iter() - .map(|user| Ok(downcast::(&user, "UserId")?.inner.clone())) - .collect() -} diff --git a/src/sync_events.rs b/src/sync_events.rs index a4d79b042..cb0ce455b 100644 --- a/src/sync_events.rs +++ b/src/sync_events.rs @@ -4,7 +4,7 @@ use js_sys::Array; use matrix_sdk_common::ruma; use wasm_bindgen::prelude::*; -use crate::{identifiers, js::downcast}; +use crate::identifiers; /// Information on E2E device updates. #[wasm_bindgen] @@ -19,20 +19,14 @@ impl DeviceLists { /// /// `changed` and `left` must be an array of `UserId`. #[wasm_bindgen(constructor)] - pub fn new(changed: Option, left: Option) -> Result { + pub fn new( + changed: Option>, + left: Option>, + ) -> Result { let mut inner = ruma::api::client::sync::sync_events::DeviceLists::default(); - inner.changed = changed - .unwrap_or_default() - .iter() - .map(|user| Ok(downcast::(&user, "UserId")?.inner.clone())) - .collect::, JsError>>()?; - - inner.left = left - .unwrap_or_default() - .iter() - .map(|user| Ok(downcast::(&user, "UserId")?.inner.clone())) - .collect::, JsError>>()?; + inner.changed = changed.unwrap_or_default().iter().map(|user| user.inner.clone()).collect(); + inner.left = left.unwrap_or_default().iter().map(|user| user.inner.clone()).collect(); Ok(Self { inner }) } diff --git a/tests/machine.test.ts b/tests/machine.test.ts index 17aaf61f6..50cc6a2af 100644 --- a/tests/machine.test.ts +++ b/tests/machine.test.ts @@ -242,7 +242,7 @@ describe(OlmMachine.name, () => { test("can update tracked users", async () => { const m = await machine(); - expect(await m.updateTrackedUsers([user])).toStrictEqual(undefined); + expect(await m.updateTrackedUsers([new UserId("@alice:example.org")])).toStrictEqual(undefined); }); test("can receive sync changes", async () => { @@ -509,9 +509,7 @@ describe(OlmMachine.name, () => { }); test("can share a room key", async () => { - const otherUsers = [new UserId("@example:localhost")]; - - const requests = await m.shareRoomKey(room, otherUsers, new EncryptionSettings()); + const requests = await m.shareRoomKey(room, [new UserId("@example:localhost")], new EncryptionSettings()); expect(requests).toHaveLength(1); expect(requests[0]).toBeInstanceOf(ToDeviceRequest); @@ -524,7 +522,8 @@ describe(OlmMachine.name, () => { expect(messageContent["org.matrix.msgid"]).toBeDefined(); await m.markRequestAsSent(requests[0].id, RequestType.ToDevice, "{}"); - const requestsAfterMarkedAsSent = await m.shareRoomKey(room, otherUsers, new EncryptionSettings()); + + const requestsAfterMarkedAsSent = await m.shareRoomKey(room, [new UserId("@example:localhost")], new EncryptionSettings()); expect(requestsAfterMarkedAsSent).toHaveLength(0); });