diff --git a/matchbox_socket/src/webrtc_socket/error.rs b/matchbox_socket/src/webrtc_socket/error.rs index c4f671f0..4e0bec19 100644 --- a/matchbox_socket/src/webrtc_socket/error.rs +++ b/matchbox_socket/src/webrtc_socket/error.rs @@ -2,14 +2,16 @@ use crate::webrtc_socket::messages::PeerEvent; use cfg_if::cfg_if; /// An error that can occur when getting a socket's channel through -/// `get_channel`, `take_channel` or `try_update_peers`. +/// [`get_channel`](crate::webrtc_socket::WebRtcSocket::get_channel), +/// [`take_channel`](crate::webrtc_socket::WebRtcSocket::take_channel) or +/// [`try_update_peers`](crate::webrtc_socket::WebRtcSocket::try_update_peers). #[derive(Debug, thiserror::Error)] pub enum ChannelError { /// Can occur if trying to get a channel with an Id that was not added while building the /// socket #[error("This channel was never created")] NotFound, - /// The channel has already been taken and is no longer on the socket + /// The channel has already been taken and is no longer on the socket. #[error("This channel has already been taken and is no longer on the socket")] Taken, /// Channel might have been opened but later closed, or never opened in the first place. diff --git a/matchbox_socket/src/webrtc_socket/socket.rs b/matchbox_socket/src/webrtc_socket/socket.rs index f10e7e72..8974eddd 100644 --- a/matchbox_socket/src/webrtc_socket/socket.rs +++ b/matchbox_socket/src/webrtc_socket/socket.rs @@ -279,6 +279,8 @@ pub enum PeerState { } /// Used to send and receive packets on a given WebRTC channel. Must be created as part of a /// [`WebRtcSocket`]. +/// This corresponds to a collection of lower level channels, one for each [`PeerState::Connected`] +/// peer (identified by their PeerId). #[derive(Debug)] pub struct WebRtcChannel { config: ChannelConfig, @@ -472,6 +474,9 @@ pub struct WebRtcSocket { id_rx: futures_channel::oneshot::Receiver, peer_state_rx: futures_channel::mpsc::UnboundedReceiver<(PeerId, PeerState)>, peers: HashMap, + /// The channels, in the order specified by the builder. + /// These are [`Some`] even before any connection is made: the only transition to [`None`] when + /// [`ChannelError::Taken`]. channels: Vec>, } @@ -711,7 +716,8 @@ impl WebRtcSocket { .ok_or(ChannelError::Taken) } - /// Takes the [`WebRtcChannel`] of a given id. + /// Takes the [`WebRtcChannel`] at the specified index. + /// The channels are indexed based on the order they were added to the builder. /// /// ``` /// use matchbox_socket::*; @@ -725,9 +731,9 @@ impl WebRtcSocket { /// ``` /// /// See also: [`WebRtcSocket::channel`] - pub fn take_channel(&mut self, channel: usize) -> Result { + pub fn take_channel(&mut self, channel_index: usize) -> Result { self.channels - .get_mut(channel) + .get_mut(channel_index) .ok_or(ChannelError::NotFound)? .take() .ok_or(ChannelError::Taken) @@ -735,12 +741,12 @@ impl WebRtcSocket { /// Takes the [`WebRtcChannel`] of a given [`PeerId`]. pub fn take_channel_by_id(&mut self, id: PeerId) -> Result { - let pos = self + let peer_index = self .connected_peers() .position(|peer_id| peer_id == id) .ok_or(ChannelError::NotFound)?; - self.take_channel(pos) + self.take_channel(peer_index) } /// Converts the [`WebRtcChannel`] of a given [`PeerId`] into a [`RawPeerChannel`]. @@ -906,7 +912,7 @@ fn compat_read_write( #[cfg(test)] mod test { - use crate::{ChannelConfig, Error, WebRtcSocketBuilder}; + use crate::{ChannelConfig, ChannelError, Error, WebRtcSocket, WebRtcSocketBuilder}; #[futures_test::test] async fn unreachable_server() { @@ -937,4 +943,43 @@ mod test { Error::ConnectionFailed { .. }, )); } + + #[test] + #[should_panic(expected = "Must have added at least one channel")] + fn no_channels() { + let (_socket, _fut) = WebRtcSocketBuilder::new("wss://example.invalid").build(); + } + + #[test] + fn builder() { + let mut builder = WebRtcSocket::builder("wss://example.invalid"); + builder = builder.add_reliable_channel(); + builder = builder.add_unreliable_channel(); + let (socket, _fut) = builder.build(); + assert_eq!(socket.channels.len(), 2); + // Channels are populated immediately, even before connecting. + let config_0 = &socket.channels[0].as_ref().unwrap().config; + let config_1 = &socket.channels[1].as_ref().unwrap().config; + assert!(config_0.ordered); + assert_eq!(config_0.max_retransmits, None); + assert!(!config_1.ordered); + assert_eq!(config_1.max_retransmits, Some(0)); + } + + #[test] + fn take_channel() { + let mut builder = WebRtcSocket::builder("wss://example.invalid"); + builder = builder.add_reliable_channel(); + let (mut socket, _fut) = builder.build(); + assert!(socket.channels[0].is_some()); + let mut taken = socket.take_channel(0).unwrap(); + assert!(socket.channels[0].is_none()); + assert!(matches!( + socket.take_channel(0).unwrap_err(), + ChannelError::Taken, + )); + assert!(!taken.is_closed()); + taken.close(); + assert!(taken.is_closed()); + } }