From ee7958a3bc0895e90ce2a6807dcbe16ee6a38ab1 Mon Sep 17 00:00:00 2001 From: Serhii Mamontov Date: Mon, 28 Aug 2023 02:07:49 +0300 Subject: [PATCH] fix: fix clippy warnings and failing tests --- src/core/utils/encoding.rs | 4 +-- src/core/utils/mod.rs | 14 ++++++-- src/dx/presence/builders/mod.rs | 7 ++-- .../event_engine/effects/heartbeat.rs | 18 ++--------- src/dx/presence/event_engine/effects/leave.rs | 16 ++-------- src/dx/presence/event_engine/event.rs | 1 + src/dx/presence/mod.rs | 32 ++++++++++--------- src/dx/presence/presence_manager.rs | 30 ++++++++++++++++- src/dx/publish/builders.rs | 7 ++-- src/dx/subscribe/builders/subscribe.rs | 2 +- src/dx/subscribe/builders/subscription.rs | 2 +- src/dx/subscribe/mod.rs | 6 ++-- 12 files changed, 80 insertions(+), 59 deletions(-) diff --git a/src/core/utils/encoding.rs b/src/core/utils/encoding.rs index 01f588d6..8dff339c 100644 --- a/src/core/utils/encoding.rs +++ b/src/core/utils/encoding.rs @@ -74,7 +74,7 @@ pub fn join_url_encoded(strings: &[&str], sep: &str) -> Option { /// URL-encode channels list. /// /// Channels list used as part of URL path and therefore required. -#[cfg(all(any(feature = "subscribe", feature = "presence"), feature = "std"))] +#[cfg(any(feature = "subscribe", feature = "presence"))] pub(crate) fn url_encoded_channels(channels: &[String]) -> String { join_url_encoded( channels @@ -88,7 +88,7 @@ pub(crate) fn url_encoded_channels(channels: &[String]) -> String { } /// URL-encode channel groups list. -#[cfg(all(any(feature = "subscribe", feature = "presence"), feature = "std"))] +#[cfg(any(feature = "subscribe", feature = "presence"))] pub(crate) fn url_encoded_channel_groups(channel_groups: &[String]) -> Option { join_url_encoded( channel_groups diff --git a/src/core/utils/mod.rs b/src/core/utils/mod.rs index 23e687c2..0a8a0625 100644 --- a/src/core/utils/mod.rs +++ b/src/core/utils/mod.rs @@ -1,6 +1,16 @@ -#[cfg(any(feature = "publish", feature = "access", feature = "subscribe"))] +#[cfg(any( + feature = "publish", + feature = "access", + feature = "subscribe", + feature = "presence" +))] pub mod encoding; -#[cfg(any(feature = "publish", feature = "access", feature = "subscribe"))] +#[cfg(any( + feature = "publish", + feature = "access", + feature = "subscribe", + feature = "presence" +))] pub mod headers; pub mod metadata; diff --git a/src/dx/presence/builders/mod.rs b/src/dx/presence/builders/mod.rs index 3f23fe91..6951d9c1 100644 --- a/src/dx/presence/builders/mod.rs +++ b/src/dx/presence/builders/mod.rs @@ -6,7 +6,7 @@ //! [`PubNub`]: https://www.pubnub.com #[doc(inline)] -pub(crate) use heartbeat::{HeartbeatRequest, HeartbeatRequestBuilder}; +pub(crate) use heartbeat::HeartbeatRequestBuilder; pub(crate) mod heartbeat; #[doc(inline)] @@ -14,7 +14,7 @@ pub use set_state::{SetStateRequest, SetStateRequestBuilder}; pub mod set_state; #[doc(inline)] -pub(crate) use leave::{LeaveRequest, LeaveRequestBuilder}; +pub(crate) use leave::LeaveRequestBuilder; pub(crate) mod leave; use crate::{dx::pubnub_client::PubNubClientInstance, lib::alloc::string::String}; @@ -28,7 +28,8 @@ pub(in crate::dx::presence::builders) fn validate_configuration( ) -> Result<(), String> { let client = client .as_ref() - .expect("PubNub client instance not set.".into()); + .unwrap_or_else(|| panic!("PubNub client instance not set.")); + if client.config.subscribe_key.is_empty() { return Err("Incomplete PubNub client configuration: 'subscribe_key' is empty.".into()); } diff --git a/src/dx/presence/event_engine/effects/heartbeat.rs b/src/dx/presence/event_engine/effects/heartbeat.rs index 1fae08dd..dbc9943b 100644 --- a/src/dx/presence/event_engine/effects/heartbeat.rs +++ b/src/dx/presence/event_engine/effects/heartbeat.rs @@ -51,10 +51,7 @@ pub(super) async fn execute( effect_id, }) .map_ok_or_else( - |error| { - log::error!("Heartbeat error: {error:?}"); - vec![PresenceEvent::HeartbeatFailure { reason: error }] - }, + |error| vec![PresenceEvent::HeartbeatFailure { reason: error }], |_| vec![PresenceEvent::HeartbeatSuccess], ) .await @@ -75,13 +72,7 @@ mod it_should { assert_eq!(parameters.channel_groups, &Some(vec!["cg1".to_string()])); assert_eq!(parameters.channels, &Some(vec!["ch1".to_string()])); assert_eq!(parameters.attempt, 0); - assert_eq!( - parameters.reason.unwrap(), - PubNubError::Transport { - details: "test".into(), - response: None - } - ); + assert_eq!(parameters.reason, None); assert_eq!(parameters.effect_id, "id"); async move { Ok(HeartbeatResult) }.boxed() @@ -93,10 +84,7 @@ mod it_should { &Some(vec!["cg1".to_string()]), ), 0, - Some(PubNubError::Transport { - details: "test".into(), - response: None, - }), + None, "id", &Some(RequestRetryPolicy::None), &mocked_heartbeat_function, diff --git a/src/dx/presence/event_engine/effects/leave.rs b/src/dx/presence/event_engine/effects/leave.rs index ed10d806..55345322 100644 --- a/src/dx/presence/event_engine/effects/leave.rs +++ b/src/dx/presence/event_engine/effects/leave.rs @@ -56,13 +56,7 @@ mod it_should { assert_eq!(parameters.channel_groups, &Some(vec!["cg2".to_string()])); assert_eq!(parameters.channels, &Some(vec!["ch2".to_string()])); assert_eq!(parameters.attempt, 0); - assert_eq!( - parameters.reason.unwrap(), - PubNubError::Transport { - details: "test".into(), - response: None - } - ); + assert_eq!(parameters.reason, None); assert_eq!(parameters.effect_id, "id"); async move { Ok(LeaveResult) }.boxed() @@ -82,7 +76,7 @@ mod it_should { } #[tokio::test] - async fn return_heartbeat_failed_event_on_error() { + async fn return_leave_failed_event_on_error() { let mocked_leave_function: Arc = Arc::new(move |_| { async move { Err(PubNubError::Transport { @@ -106,10 +100,6 @@ mod it_should { ) .await; - assert!(!result.is_empty()); - assert!(matches!( - result.first().unwrap(), - PresenceEvent::HeartbeatFailure { .. } - )); + assert!(result.is_empty()); } } diff --git a/src/dx/presence/event_engine/event.rs b/src/dx/presence/event_engine/event.rs index e89ae136..5e50fb1e 100644 --- a/src/dx/presence/event_engine/event.rs +++ b/src/dx/presence/event_engine/event.rs @@ -5,6 +5,7 @@ use crate::core::{event_engine::Event, PubNubError}; +#[derive(Debug)] pub(crate) enum PresenceEvent { /// Announce join to channels and groups. /// diff --git a/src/dx/presence/mod.rs b/src/dx/presence/mod.rs index a21c2271..fcac3fcc 100644 --- a/src/dx/presence/mod.rs +++ b/src/dx/presence/mod.rs @@ -26,7 +26,7 @@ pub mod result; #[cfg(feature = "std")] #[doc(inline)] -pub(crate) use presence_manager::{PresenceManager, PresenceManagerRef}; +pub(crate) use presence_manager::PresenceManager; #[cfg(feature = "std")] pub(crate) mod presence_manager; #[cfg(feature = "std")] @@ -181,16 +181,17 @@ where #[allow(dead_code)] pub(crate) fn announce_join( &self, - _channels: Option>, - _channel_groups: Option>, + channels: Option>, + channel_groups: Option>, ) { self.configure_presence(); - // if let Some(presence) = self.presence.clone().write().as_mut() { - // params - // .state - // .map(|state| presence.state = Some(Arc::new(state))); - // } + { + let slot = self.presence.read(); + if let Some(presence) = slot.as_ref() { + presence.announce_join(channels, channel_groups); + } + }; } /// Announce `leave` for `user_id` on provided channels and groups. @@ -198,16 +199,17 @@ where #[allow(dead_code)] pub(crate) fn announce_left( &self, - _channels: Option>, - _channel_groups: Option>, + channels: Option>, + channel_groups: Option>, ) { self.configure_presence(); - // if let Some(presence) = self.presence.clone().write().as_mut() { - // params - // .state - // .map(|state| presence.state = Some(Arc::new(state))); - // } + { + let slot = self.presence.read(); + if let Some(presence) = slot.as_ref() { + presence.announce_left(channels, channel_groups); + } + }; } /// Complete presence configuration. diff --git a/src/dx/presence/presence_manager.rs b/src/dx/presence/presence_manager.rs index 1a198d6a..3cc3a52c 100644 --- a/src/dx/presence/presence_manager.rs +++ b/src/dx/presence/presence_manager.rs @@ -4,7 +4,7 @@ //! presence / heartbeat module components. use crate::{ - dx::presence::event_engine::PresenceEventEngine, + dx::presence::event_engine::{PresenceEvent, PresenceEventEngine}, lib::{ alloc::sync::Arc, core::{ @@ -86,6 +86,34 @@ pub(crate) struct PresenceManagerRef { pub state: Option>, } +impl PresenceManagerRef { + /// Announce `join` for `user_id` on provided channels and groups. + #[allow(dead_code)] + pub(crate) fn announce_join( + &self, + channels: Option>, + channel_groups: Option>, + ) { + self.event_engine.process(&PresenceEvent::Joined { + channels, + channel_groups, + }) + } + + /// Announce `leave` for `user_id` on provided channels and groups. + #[allow(dead_code)] + pub(crate) fn announce_left( + &self, + channels: Option>, + channel_groups: Option>, + ) { + self.event_engine.process(&PresenceEvent::Left { + channels, + channel_groups, + }) + } +} + impl Debug for PresenceManagerRef { fn fmt(&self, f: &mut Formatter<'_>) -> Result { write!( diff --git a/src/dx/publish/builders.rs b/src/dx/publish/builders.rs index ee285e7f..614fa0dd 100644 --- a/src/dx/publish/builders.rs +++ b/src/dx/publish/builders.rs @@ -1,16 +1,15 @@ -//! Publish builders module. +//! # Publish builders module. //! //! This module contains all builders for the publish operation. +use derive_builder::Builder; + use crate::{ core::Serialize, dx::pubnub_client::PubNubClientInstance, lib::{alloc::string::String, collections::HashMap}, }; -use crate::core::Transport; -use derive_builder::Builder; - /// The [`PublishMessageBuilder`] is used to publish a message to a channel. /// /// This struct is used by the [`publish_message`] method of the diff --git a/src/dx/subscribe/builders/subscribe.rs b/src/dx/subscribe/builders/subscribe.rs index 608ed38c..8378ef9b 100644 --- a/src/dx/subscribe/builders/subscribe.rs +++ b/src/dx/subscribe/builders/subscribe.rs @@ -152,7 +152,7 @@ impl SubscribeRequest { // Serialize list of channel groups and add into query parameters list. url_encoded_channel_groups(&self.channel_groups) - .and_then(|groups| query.insert("channel-group".into(), groups.into())); + .and_then(|groups| query.insert("channel-group".into(), groups)); self.filter_expression .as_ref() diff --git a/src/dx/subscribe/builders/subscription.rs b/src/dx/subscribe/builders/subscription.rs index db446504..61295cce 100644 --- a/src/dx/subscribe/builders/subscription.rs +++ b/src/dx/subscribe/builders/subscription.rs @@ -245,7 +245,7 @@ impl SubscriptionBuilder { let input = self .input .as_ref() - .expect("Subscription input should be set by default".into()); + .unwrap_or_else(|| panic!("Subscription input should be set by default")); if input.is_empty { return Err("Either channels or channel groups should be provided".into()); diff --git a/src/dx/subscribe/mod.rs b/src/dx/subscribe/mod.rs index 4a14579f..64120c40 100644 --- a/src/dx/subscribe/mod.rs +++ b/src/dx/subscribe/mod.rs @@ -7,6 +7,7 @@ use futures::{ future::{ready, BoxFuture}, FutureExt, }; +#[cfg(feature = "std")] use spin::RwLock; use crate::dx::{pubnub_client::PubNubClientInstance, subscribe::raw::RawSubscriptionBuilder}; @@ -43,10 +44,11 @@ pub mod result; pub(crate) use subscription_manager::SubscriptionManager; #[cfg(feature = "std")] pub(crate) mod subscription_manager; -use crate::subscribe::event_engine::SubscribeEventEngine; #[cfg(feature = "std")] #[doc(inline)] -use event_engine::{types::SubscriptionParams, SubscribeEffectHandler, SubscribeState}; +use event_engine::{ + types::SubscriptionParams, SubscribeEffectHandler, SubscribeEventEngine, SubscribeState, +}; #[cfg(feature = "std")] pub(crate) mod event_engine;