From d213ee471814f28c398fb7057b2736a45124378c Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Sat, 3 Jul 2021 00:29:58 +0200 Subject: [PATCH] events: Remove / replace previous error types --- crates/ruma-events/CHANGELOG.md | 9 ++ crates/ruma-events/Cargo.toml | 1 + crates/ruma-events/src/error.rs | 29 ---- .../ruma-events/src/key/verification/start.rs | 141 ------------------ crates/ruma-events/src/lib.rs | 7 +- crates/ruma-events/src/room/name.rs | 27 +++- 6 files changed, 32 insertions(+), 182 deletions(-) delete mode 100644 crates/ruma-events/src/error.rs diff --git a/crates/ruma-events/CHANGELOG.md b/crates/ruma-events/CHANGELOG.md index da60189f..227c21d9 100644 --- a/crates/ruma-events/CHANGELOG.md +++ b/crates/ruma-events/CHANGELOG.md @@ -1,5 +1,14 @@ # [unreleased] +Breaking changes: + +* `room::name::NameEventContent` now uses a custom `RoomName` type for its + `name` field and makes it public, in response the constructor and `name` + accessor had their types updated too +* Replace `InvalidEvent` by a more specific `FromStringError` for room name + validation +* Remove unused `FromStrError` + # 0.23.3 Improvements: diff --git a/crates/ruma-events/Cargo.toml b/crates/ruma-events/Cargo.toml index 8f80d2e0..74dc8205 100644 --- a/crates/ruma-events/Cargo.toml +++ b/crates/ruma-events/Cargo.toml @@ -33,6 +33,7 @@ ruma-identifiers = { version = "0.19.4", path = "../ruma-identifiers", features ruma-serde = { version = "0.4.2", path = "../ruma-serde" } serde = { version = "1.0.118", features = ["derive"] } serde_json = { version = "1.0.60", features = ["raw_value"] } +thiserror = "1.0.25" [dev-dependencies] assign = "1.1.1" diff --git a/crates/ruma-events/src/error.rs b/crates/ruma-events/src/error.rs deleted file mode 100644 index d2cd1672..00000000 --- a/crates/ruma-events/src/error.rs +++ /dev/null @@ -1,29 +0,0 @@ -use std::{error::Error, fmt}; - -/// An error returned when attempting to create an event with data that would make it invalid. -/// -/// This type is similar to [`InvalidEvent`](struct.InvalidEvent.html), but used during the -/// construction of a new event, as opposed to deserialization of an existing event from JSON. -#[derive(Clone, Debug, PartialEq)] -pub struct InvalidInput(pub(crate) String); - -impl fmt::Display for InvalidInput { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) - } -} - -impl Error for InvalidInput {} - -/// An error when attempting to create a value from a string via the `FromStr` trait. -#[derive(Clone, Eq, Debug, Hash, PartialEq)] -#[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)] -pub struct FromStrError; - -impl fmt::Display for FromStrError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "failed to parse type from string") - } -} - -impl Error for FromStrError {} diff --git a/crates/ruma-events/src/key/verification/start.rs b/crates/ruma-events/src/key/verification/start.rs index 0d033c63..b1d02643 100644 --- a/crates/ruma-events/src/key/verification/start.rs +++ b/crates/ruma-events/src/key/verification/start.rs @@ -572,145 +572,4 @@ mod tests { && secret == "It's a secret to everybody" ); } - - #[test] - fn deserialization_failure() { - // Ensure that invalid JSON Creates a new `serde_json::Error` and not `InvalidEvent` - assert!(serde_json::from_str::>("{").is_err()); - } - - // TODO this fails because the error is a Validation error not deserialization? - /* - #[test] - fn deserialization_structure_mismatch() { - // Missing several required fields. - let error = - from_json_value::>(json!({ "from_device": "123" })) - .unwrap() - .deserialize() - .unwrap_err(); - - assert!(error.message().contains("missing field")); - assert!(error.is_deserialization()); - } - */ - - // TODO re implement validation done in TryFromRaw else where - /* - #[test] - fn deserialization_validation_missing_required_key_agreement_protocols() { - let json_data = json!({ - "from_device": "123", - "transaction_id": "456", - "method": "m.sas.v1", - "key_agreement_protocols": [], - "hashes": ["sha256"], - "message_authentication_codes": ["hkdf-hmac-sha256"], - "short_authentication_string": ["decimal"] - }); - - let error = from_json_value::>(json_data) - .unwrap() - .deserialize() - .unwrap_err(); - - assert!(error.message().contains("key_agreement_protocols")); - assert!(error.is_validation()); - } - */ - - // TODO re implement validation done in TryFromRaw else where - /* - #[test] - fn deserialization_validation_missing_required_hashes() { - let json_data = json!({ - "from_device": "123", - "transaction_id": "456", - "method": "m.sas.v1", - "key_agreement_protocols": ["curve25519"], - "hashes": [], - "message_authentication_codes": ["hkdf-hmac-sha256"], - "short_authentication_string": ["decimal"] - }); - let error = from_json_value::>(json_data) - .unwrap() - .deserialize() - .unwrap_err(); - - assert!(error.message().contains("hashes")); - assert!(error.is_validation()); - } - */ - - // TODO re implement validation done in TryFromRaw else where - /* - #[test] - fn deserialization_validation_missing_required_message_authentication_codes() { - let json_data = json!({ - "from_device": "123", - "transaction_id": "456", - "method": "m.sas.v1", - "key_agreement_protocols": ["curve25519"], - "hashes": ["sha256"], - "message_authentication_codes": [], - "short_authentication_string": ["decimal"] - }); - let error = from_json_value::>(json_data) - .unwrap() - .deserialize() - .unwrap_err(); - - assert!(error.message().contains("message_authentication_codes")); - assert!(error.is_validation()); - } - */ - - /* - #[test] - fn deserialization_validation_missing_required_short_authentication_string() { - let json_data = json!({ - "from_device": "123", - "transaction_id": "456", - "method": "m.sas.v1", - "key_agreement_protocols": ["curve25519"], - "hashes": ["sha256"], - "message_authentication_codes": ["hkdf-hmac-sha256"], - "short_authentication_string": [] - }); - let error = from_json_value::>(json_data) - .unwrap() - .deserialize() - .unwrap_err(); - - assert!(error.message().contains("short_authentication_string")); - assert!(error.is_validation()); - } - */ - - // TODO re implement validation done in TryFromRaw else where - /* - #[test] - fn deserialization_of_event_validates_content() { - // This JSON is missing the required value of "curve25519" for "key_agreement_protocols". - let json_data = json!({ - "content": { - "from_device": "123", - "transaction_id": "456", - "method": "m.sas.v1", - "key_agreement_protocols": [], - "hashes": ["sha256"], - "message_authentication_codes": ["hkdf-hmac-sha256"], - "short_authentication_string": ["decimal"] - }, - "type": "m.key.verification.start" - }); - let error = from_json_value::>(json_data) - .unwrap() - .deserialize() - .unwrap_err(); - - assert!(error.message().contains("key_agreement_protocols")); - assert!(error.is_validation()); - } - **/ } diff --git a/crates/ruma-events/src/lib.rs b/crates/ruma-events/src/lib.rs index ab8f3d2d..1e361f40 100644 --- a/crates/ruma-events/src/lib.rs +++ b/crates/ruma-events/src/lib.rs @@ -129,7 +129,6 @@ use serde_json::value::RawValue as RawJsonValue; use self::room::redaction::SyncRedactionEvent; mod enums; -mod error; mod event_kinds; // Hack to allow both ruma-events itself and external crates (or tests) to use procedural macros @@ -186,11 +185,7 @@ pub mod typing; #[cfg(feature = "unstable-pre-spec")] #[cfg_attr(docsrs, doc(cfg(feature = "unstable-pre-spec")))] pub use self::relation::Relations; -pub use self::{ - enums::*, - error::{FromStrError, InvalidInput}, - event_kinds::*, -}; +pub use self::{enums::*, event_kinds::*}; /// Extra information about an event that is not incorporated into the event's /// hash. diff --git a/crates/ruma-events/src/room/name.rs b/crates/ruma-events/src/room/name.rs index dd2530aa..3d57da88 100644 --- a/crates/ruma-events/src/room/name.rs +++ b/crates/ruma-events/src/room/name.rs @@ -4,8 +4,9 @@ use std::convert::TryFrom; use ruma_events_macros::EventContent; use serde::{Deserialize, Serialize}; +use thiserror::Error; -use crate::{InvalidInput, StateEvent}; +use crate::StateEvent; /// The room name is a human-friendly string designed to be displayed to the end-user. pub type NameEvent = StateEvent; @@ -27,6 +28,7 @@ impl NameEventContent { } /// The name of the room, if any. + #[deprecated = "You can access the name field directly."] pub fn name(&self) -> Option<&RoomName> { self.name.as_ref() } @@ -34,19 +36,19 @@ impl NameEventContent { /// The name of a room. /// -/// It should not exceed 255 characters and should not be empty. +/// It can't exceed 255 characters or be empty. #[derive(Clone, Debug, Serialize, PartialEq, Eq)] #[serde(transparent)] pub struct RoomName(String); impl TryFrom for RoomName { - type Error = InvalidInput; + type Error = FromStringError; fn try_from(value: String) -> Result { match value.len() { - 0 => Err(InvalidInput("a room name cannot be empty.".into())), + 0 => Err(FromStringError::Empty), 1..=255 => Ok(RoomName(value)), - _ => Err(InvalidInput("a room name cannot be more than 255 bytes.".into())), + _ => Err(FromStringError::TooLong), } } } @@ -68,11 +70,24 @@ impl<'de> Deserialize<'de> for RoomName { match RoomName::try_from(str_name) { Ok(name) => Ok(name), - Err(e) => Err(D::Error::custom(e.to_string())), + Err(e) => Err(D::Error::custom(e)), } } } +/// Errors that can occur when converting a string to `RoomName`. +#[derive(Debug, Error)] +#[non_exhaustive] +pub enum FromStringError { + /// Room name string was empty. + #[error("room name may not be empty")] + Empty, + + /// Room name string exceeded 255 byte limit. + #[error("room name length may not exceed 255 bytes")] + TooLong, +} + #[cfg(test)] mod tests { use std::convert::TryFrom;