events: Use a string for SessionDescription's type

A clarification in MSC2746 / Matrix 1.7 explains that the `type` field
should not be validated but passed as-is to the WebRTC API.
It
    also avoids an unnecessary conversion between the WebRTC API
and the Ruma type.
This commit is contained in:
Kévin Commaille 2023-05-26 11:39:24 +02:00 committed by Kévin Commaille
parent 60ed2c7b9a
commit 3f28f2a6f1
5 changed files with 33 additions and 91 deletions

View File

@ -13,6 +13,11 @@ Breaking changes:
- Remove the `DontNotify` and `Coalesce` variants of `push::Action` according to MSC3987 - Remove the `DontNotify` and `Coalesce` variants of `push::Action` according to MSC3987
- Old push rules will still deserialize successfully but the `Coalesce` variant will not return - Old push rules will still deserialize successfully but the `Coalesce` variant will not return
`true` for `Action::should_notify()` anymore `true` for `Action::should_notify()` anymore
- Remove `AnswerSessionDescription` and `OfferSessionDescription` types, use `SessionDescription`
instead.
- Remove `SessionDescriptionType`, use a `String` instead. A clarification in MSC2746 / Matrix 1.7
explains that the `type` field should not be validated but passed as-is to the WebRTC API. It
also avoids an unnecessary conversion between the WebRTC API and the Ruma type.
Improvements: Improvements:

View File

@ -15,8 +15,6 @@ pub mod select_answer;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use crate::{serde::StringEnum, PrivOwnedStr};
/// A VoIP session description. /// A VoIP session description.
/// ///
/// This is the same type as WebRTC's [`RTCSessionDescriptionInit`]. /// This is the same type as WebRTC's [`RTCSessionDescriptionInit`].
@ -26,8 +24,10 @@ use crate::{serde::StringEnum, PrivOwnedStr};
#[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)] #[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
pub struct SessionDescription { pub struct SessionDescription {
/// The type of session description. /// The type of session description.
///
/// This is the `type` field of `RTCSessionDescriptionInit`.
#[serde(rename = "type")] #[serde(rename = "type")]
pub session_type: SessionDescriptionType, pub session_type: String,
/// The SDP text of the session description. /// The SDP text of the session description.
/// ///
@ -39,73 +39,11 @@ pub struct SessionDescription {
impl SessionDescription { impl SessionDescription {
/// Creates a new `SessionDescription` with the given session type and SDP text. /// Creates a new `SessionDescription` with the given session type and SDP text.
pub fn new(session_type: SessionDescriptionType, sdp: String) -> Self { pub fn new(session_type: String, sdp: String) -> Self {
Self { session_type, sdp } Self { session_type, sdp }
} }
} }
/// The type of VoIP session description.
///
/// This is the same type as WebRTC's [`RTCSdpType`].
///
/// [`RTCSdpType`]: (https://www.w3.org/TR/webrtc/#dom-rtcsdptype):
#[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/doc/string_enum.md"))]
#[derive(Clone, PartialEq, Eq, StringEnum)]
#[ruma_enum(rename_all = "lowercase")]
#[non_exhaustive]
pub enum SessionDescriptionType {
/// The description must be treated as an SDP final answer, and the offer-answer exchange must
/// be considered complete.
Answer,
/// The description must be treated as an SDP offer.
Offer,
/// The description must be treated as an SDP answer, but not final.
#[cfg(feature = "unstable-msc2746")]
PrAnswer,
/// The description must be treated as cancelling the current SDP negotiation and moving the
/// SDP offer back to what it was in the previous stable state.
#[cfg(feature = "unstable-msc2746")]
Rollback,
#[doc(hidden)]
_Custom(PrivOwnedStr),
}
/// A VoIP answer session description.
#[derive(Clone, Debug, Deserialize, Serialize)]
#[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
#[serde(tag = "type", rename = "answer")]
pub struct AnswerSessionDescription {
/// The SDP text of the session description.
pub sdp: String,
}
impl AnswerSessionDescription {
/// Creates a new `AnswerSessionDescription` with the given SDP text.
pub fn new(sdp: String) -> Self {
Self { sdp }
}
}
/// A VoIP offer session description.
#[derive(Clone, Debug, Deserialize, Serialize)]
#[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
#[serde(tag = "type", rename = "offer")]
pub struct OfferSessionDescription {
/// The SDP text of the session description.
pub sdp: String,
}
impl OfferSessionDescription {
/// Creates a new `OfferSessionDescription` with the given SDP text.
pub fn new(sdp: String) -> Self {
Self { sdp }
}
}
/// The capabilities of a client in a VoIP call. /// The capabilities of a client in a VoIP call.
#[cfg(feature = "unstable-msc2747")] #[cfg(feature = "unstable-msc2747")]
#[derive(Clone, Debug, Default, Serialize, Deserialize)] #[derive(Clone, Debug, Default, Serialize, Deserialize)]

View File

@ -5,9 +5,9 @@
use ruma_macros::EventContent; use ruma_macros::EventContent;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use super::AnswerSessionDescription;
#[cfg(feature = "unstable-msc2747")] #[cfg(feature = "unstable-msc2747")]
use super::CallCapabilities; use super::CallCapabilities;
use super::SessionDescription;
use crate::{OwnedVoipId, VoipVersionId}; use crate::{OwnedVoipId, VoipVersionId};
/// The content of an `m.call.answer` event. /// The content of an `m.call.answer` event.
@ -18,7 +18,7 @@ use crate::{OwnedVoipId, VoipVersionId};
#[ruma_event(type = "m.call.answer", kind = MessageLike)] #[ruma_event(type = "m.call.answer", kind = MessageLike)]
pub struct CallAnswerEventContent { pub struct CallAnswerEventContent {
/// The VoIP session description object. /// The VoIP session description object.
pub answer: AnswerSessionDescription, pub answer: SessionDescription,
/// A unique identifier for the call. /// A unique identifier for the call.
pub call_id: OwnedVoipId, pub call_id: OwnedVoipId,
@ -39,11 +39,7 @@ pub struct CallAnswerEventContent {
impl CallAnswerEventContent { impl CallAnswerEventContent {
/// Creates an `CallAnswerEventContent` with the given answer, call ID and VoIP version. /// Creates an `CallAnswerEventContent` with the given answer, call ID and VoIP version.
pub fn new( pub fn new(answer: SessionDescription, call_id: OwnedVoipId, version: VoipVersionId) -> Self {
answer: AnswerSessionDescription,
call_id: OwnedVoipId,
version: VoipVersionId,
) -> Self {
Self { Self {
answer, answer,
call_id, call_id,
@ -57,7 +53,7 @@ impl CallAnswerEventContent {
/// Convenience method to create a VoIP version 0 `CallAnswerEventContent` with all the required /// Convenience method to create a VoIP version 0 `CallAnswerEventContent` with all the required
/// fields. /// fields.
pub fn version_0(answer: AnswerSessionDescription, call_id: OwnedVoipId) -> Self { pub fn version_0(answer: SessionDescription, call_id: OwnedVoipId) -> Self {
Self::new(answer, call_id, VoipVersionId::V0) Self::new(answer, call_id, VoipVersionId::V0)
} }
@ -65,7 +61,7 @@ impl CallAnswerEventContent {
/// fields. /// fields.
#[cfg(feature = "unstable-msc2746")] #[cfg(feature = "unstable-msc2746")]
pub fn version_1( pub fn version_1(
answer: AnswerSessionDescription, answer: SessionDescription,
call_id: OwnedVoipId, call_id: OwnedVoipId,
party_id: OwnedVoipId, party_id: OwnedVoipId,
) -> Self { ) -> Self {

View File

@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize};
#[cfg(feature = "unstable-msc2747")] #[cfg(feature = "unstable-msc2747")]
use super::CallCapabilities; use super::CallCapabilities;
use super::OfferSessionDescription; use super::SessionDescription;
#[cfg(feature = "unstable-msc2746")] #[cfg(feature = "unstable-msc2746")]
use crate::OwnedUserId; use crate::OwnedUserId;
use crate::{OwnedVoipId, VoipVersionId}; use crate::{OwnedVoipId, VoipVersionId};
@ -35,7 +35,7 @@ pub struct CallInviteEventContent {
pub lifetime: UInt, pub lifetime: UInt,
/// The session description object. /// The session description object.
pub offer: OfferSessionDescription, pub offer: SessionDescription,
/// The version of the VoIP specification this messages adheres to. /// The version of the VoIP specification this messages adheres to.
pub version: VoipVersionId, pub version: VoipVersionId,
@ -61,7 +61,7 @@ impl CallInviteEventContent {
pub fn new( pub fn new(
call_id: OwnedVoipId, call_id: OwnedVoipId,
lifetime: UInt, lifetime: UInt,
offer: OfferSessionDescription, offer: SessionDescription,
version: VoipVersionId, version: VoipVersionId,
) -> Self { ) -> Self {
Self { Self {
@ -80,7 +80,7 @@ impl CallInviteEventContent {
/// Convenience method to create a version 0 `CallInviteEventContent` with all the required /// Convenience method to create a version 0 `CallInviteEventContent` with all the required
/// fields. /// fields.
pub fn version_0(call_id: OwnedVoipId, lifetime: UInt, offer: OfferSessionDescription) -> Self { pub fn version_0(call_id: OwnedVoipId, lifetime: UInt, offer: SessionDescription) -> Self {
Self::new(call_id, lifetime, offer, VoipVersionId::V0) Self::new(call_id, lifetime, offer, VoipVersionId::V0)
} }
@ -91,7 +91,7 @@ impl CallInviteEventContent {
call_id: OwnedVoipId, call_id: OwnedVoipId,
party_id: OwnedVoipId, party_id: OwnedVoipId,
lifetime: UInt, lifetime: UInt,
offer: OfferSessionDescription, offer: SessionDescription,
) -> Self { ) -> Self {
Self { Self {
call_id, call_id,

View File

@ -8,7 +8,7 @@ use ruma_common::{
candidates::{CallCandidatesEventContent, Candidate}, candidates::{CallCandidatesEventContent, Candidate},
hangup::CallHangupEventContent, hangup::CallHangupEventContent,
invite::CallInviteEventContent, invite::CallInviteEventContent,
AnswerSessionDescription, OfferSessionDescription, SessionDescription,
}, },
AnyMessageLikeEvent, AnySyncMessageLikeEvent, MessageLikeEvent, AnyMessageLikeEvent, AnySyncMessageLikeEvent, MessageLikeEvent,
}, },
@ -21,7 +21,7 @@ use serde_json::{from_value as from_json_value, json, to_value as to_json_value}
#[test] #[test]
fn answer_content_serialization() { fn answer_content_serialization() {
let event_content = CallAnswerEventContent::version_0( let event_content = CallAnswerEventContent::version_0(
AnswerSessionDescription::new("not a real sdp".to_owned()), SessionDescription::new("answer".to_owned(), "not a real sdp".to_owned()),
"abcdef".into(), "abcdef".into(),
); );
@ -51,6 +51,7 @@ fn answer_content_deserialization() {
let content = from_json_value::<CallAnswerEventContent>(json_data).unwrap(); let content = from_json_value::<CallAnswerEventContent>(json_data).unwrap();
assert_eq!(content.answer.session_type, "answer");
assert_eq!(content.answer.sdp, "Hello"); assert_eq!(content.answer.sdp, "Hello");
assert_eq!(content.call_id, "foofoo"); assert_eq!(content.call_id, "foofoo");
assert_eq!(content.version, VoipVersionId::V0); assert_eq!(content.version, VoipVersionId::V0);
@ -85,6 +86,7 @@ fn answer_event_deserialization() {
assert!(message_event.unsigned.is_empty()); assert!(message_event.unsigned.is_empty());
let content = message_event.content; let content = message_event.content;
assert_eq!(content.answer.session_type, "answer");
assert_eq!(content.answer.sdp, "Hello"); assert_eq!(content.answer.sdp, "Hello");
assert_eq!(content.call_id, "foofoo"); assert_eq!(content.call_id, "foofoo");
assert_eq!(content.version, VoipVersionId::V0); assert_eq!(content.version, VoipVersionId::V0);
@ -131,7 +133,7 @@ fn invite_content_serialization() {
let event_content = CallInviteEventContent::version_0( let event_content = CallInviteEventContent::version_0(
"abcdef".into(), "abcdef".into(),
uint!(30000), uint!(30000),
OfferSessionDescription::new("not a real sdp".to_owned()), SessionDescription::new("offer".to_owned(), "not a real sdp".to_owned()),
); );
assert_eq!( assert_eq!(
@ -203,8 +205,7 @@ mod msc2746 {
negotiate::CallNegotiateEventContent, negotiate::CallNegotiateEventContent,
reject::CallRejectEventContent, reject::CallRejectEventContent,
select_answer::CallSelectAnswerEventContent, select_answer::CallSelectAnswerEventContent,
AnswerSessionDescription, OfferSessionDescription, SessionDescription, SessionDescription,
SessionDescriptionType,
}, },
AnyMessageLikeEvent, MessageLikeEvent, AnyMessageLikeEvent, MessageLikeEvent,
}, },
@ -218,7 +219,7 @@ mod msc2746 {
"abcdef".into(), "abcdef".into(),
"9876".into(), "9876".into(),
uint!(60000), uint!(60000),
OfferSessionDescription::new("not a real sdp".to_owned()), SessionDescription::new("offer".to_owned(), "not a real sdp".to_owned()),
); );
assert_eq!( assert_eq!(
@ -266,13 +267,14 @@ mod msc2746 {
assert_eq!(content.party_id.unwrap(), "9876"); assert_eq!(content.party_id.unwrap(), "9876");
assert_eq!(content.lifetime, uint!(60000)); assert_eq!(content.lifetime, uint!(60000));
assert_eq!(content.version, VoipVersionId::V1); assert_eq!(content.version, VoipVersionId::V1);
assert_eq!(content.offer.session_type, "offer");
assert_eq!(content.offer.sdp, "not a real sdp"); assert_eq!(content.offer.sdp, "not a real sdp");
} }
#[test] #[test]
fn answer_event_serialization() { fn answer_event_serialization() {
let content = CallAnswerEventContent::version_1( let content = CallAnswerEventContent::version_1(
AnswerSessionDescription::new("not a real sdp".to_owned()), SessionDescription::new("answer".to_owned(), "not a real sdp".to_owned()),
"abcdef".into(), "abcdef".into(),
"9876".into(), "9876".into(),
); );
@ -296,7 +298,7 @@ mod msc2746 {
fn answer_event_capabilities_serialization() { fn answer_event_capabilities_serialization() {
let content = assign!( let content = assign!(
CallAnswerEventContent::version_1( CallAnswerEventContent::version_1(
AnswerSessionDescription::new("not a real sdp".to_owned()), SessionDescription::new("answer".to_owned(), "not a real sdp".to_owned()),
"abcdef".into(), "abcdef".into(),
"9876".into() "9876".into()
), ),
@ -353,6 +355,7 @@ mod msc2746 {
assert_eq!(content.call_id, "abcdef"); assert_eq!(content.call_id, "abcdef");
assert_eq!(content.party_id.unwrap(), "9876"); assert_eq!(content.party_id.unwrap(), "9876");
assert_eq!(content.version.as_ref(), "org.matrix.1b"); assert_eq!(content.version.as_ref(), "org.matrix.1b");
assert_eq!(content.answer.session_type, "answer");
assert_eq!(content.answer.sdp, "not a real sdp"); assert_eq!(content.answer.sdp, "not a real sdp");
#[cfg(feature = "unstable-msc2747")] #[cfg(feature = "unstable-msc2747")]
assert!(content.capabilities.dtmf); assert!(content.capabilities.dtmf);
@ -485,7 +488,7 @@ mod msc2746 {
"abcdef".into(), "abcdef".into(),
"9876".into(), "9876".into(),
uint!(30000), uint!(30000),
SessionDescription::new(SessionDescriptionType::Offer, "not a real sdp".to_owned()), SessionDescription::new("offer".to_owned(), "not a real sdp".to_owned()),
); );
assert_eq!( assert_eq!(
@ -512,7 +515,7 @@ mod msc2746 {
"version": "1", "version": "1",
"lifetime": 30000, "lifetime": 30000,
"description": { "description": {
"type": "pranswer", "type": "answer",
"sdp": "not a real sdp", "sdp": "not a real sdp",
} }
}, },
@ -532,7 +535,7 @@ mod msc2746 {
assert_eq!(content.call_id, "abcdef"); assert_eq!(content.call_id, "abcdef");
assert_eq!(content.party_id, "9876"); assert_eq!(content.party_id, "9876");
assert_eq!(content.lifetime, uint!(30000)); assert_eq!(content.lifetime, uint!(30000));
assert_eq!(content.description.session_type, SessionDescriptionType::PrAnswer); assert_eq!(content.description.session_type, "answer");
assert_eq!(content.description.sdp, "not a real sdp"); assert_eq!(content.description.sdp, "not a real sdp");
} }