Skip to content

Commit

Permalink
Fix travis build (#65)
Browse files Browse the repository at this point in the history
* Changed `assert_eq!` tests to `assert!` to fix the build.
* Added semicolons to last line of functions to make pedantic clippy happy.
* Changed explicit impl of Default for timetoken.rs to be derived instead.
* Refactored assertion in subscription.rs to make clippy happy.
* Allowing unknown lints so that the build can pass. Older versions use `broken_intra_doc_links` and newer ones use `rustdoc::broken_intra_doc_links`, so the build fails on one or the other, so allowing unknown lints for now.
* Formatting to make the linter happy.
* Allowing unused async, because it's required for `ControlOutcome`, but Clippy insists it isn't.
* Resolving issues detected by Clippy: "expression borrows a reference...that is immediately dereferenced by the compiler"
* Bumping minimum version to Rust 1.49.0 due to:
- rust-lang/rust#55002
- rust-lang/rust#70921
* Added TODOs to remove linter allows for unknown, renamed, and removed lints when Rust 1.59.0 becomes minimum version.
* Added todo where the default UUID behavior is specified.
  • Loading branch information
stmpjmpr authored Feb 25, 2022
1 parent 859c4b0 commit ef8379e
Show file tree
Hide file tree
Showing 21 changed files with 88 additions and 75 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
language: rust
rust:
- 1.46.0
- 1.49.0
- stable
- beta
- nightly
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ The PubNub Rust SDK is based on Tokio `1.0`. This library uses `HTTP/2` to commu

## MSRV

Supports Rust 1.46.0 and higher.
Supports Rust 1.49.0 and higher.

## Get Started

Expand Down
32 changes: 16 additions & 16 deletions pubnub-core/src/data/channel/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,35 +80,35 @@ mod tests {
#[test]
fn valid() {
// Spec.
assert_eq!(is_valid(""), true);
assert_eq!(is_valid("qwe"), true);
assert_eq!(is_valid("123"), true);
assert!(is_valid(""));
assert!(is_valid("qwe"));
assert!(is_valid("123"));
}

#[test]
fn valid_but_not_officially() {
// Spec.
assert_eq!(is_valid("/"), true);
assert_eq!(is_valid("\\"), true);
assert_eq!(is_valid("."), true);
assert_eq!(is_valid("*"), true);
assert_eq!(is_valid(":"), true);
assert!(is_valid("/"));
assert!(is_valid("\\"));
assert!(is_valid("."));
assert!(is_valid("*"));
assert!(is_valid(":"));

// Real world examples.
assert_eq!(is_valid("a.b"), true);
assert_eq!(is_valid("a:b"), true);
assert_eq!(is_valid("a/b"), true);
assert_eq!(is_valid("\\a"), true);
assert_eq!(is_valid("channels_.*"), true);
assert_eq!(is_valid("channels_*"), true);
assert!(is_valid("a.b"));
assert!(is_valid("a:b"));
assert!(is_valid("a/b"));
assert!(is_valid("\\a"));
assert!(is_valid("channels_.*"));
assert!(is_valid("channels_*"));
}

#[test]
fn invalid() {
// Spec.
assert_eq!(is_valid(","), false);
assert!(!is_valid(","));

// Real world examples.
assert_eq!(is_valid("a,b"), false);
assert!(!is_valid("a,b"));
}
}
28 changes: 14 additions & 14 deletions pubnub-core/src/data/channel/wildcard_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,18 +130,18 @@ mod tests {

#[test]
fn valid() {
assert_eq!(is_valid("stocks.*"), true); // from https://support.pubnub.com/support/solutions/articles/14000043663-how-do-i-subscribe-to-a-wildcard-channel-
assert_eq!(is_valid(""), true);
assert!(is_valid("stocks.*")); // from https://support.pubnub.com/support/solutions/articles/14000043663-how-do-i-subscribe-to-a-wildcard-channel-
assert!(is_valid(""));
}

#[test]
fn valid_from_docs() {
// From https://support.pubnub.com/support/solutions/articles/14000043664-how-many-channel-segments-are-supported-with-wildcard-subscribe-

assert_eq!(is_valid("a.*"), true);
assert_eq!(is_valid("a.b.*"), true);
assert_eq!(is_valid("a.b"), true);
assert_eq!(is_valid("a.b.c"), true);
assert!(is_valid("a.*"));
assert!(is_valid("a.b.*"));
assert!(is_valid("a.b"));
assert!(is_valid("a.b.c"));

// Technically speaking, the last two examples are just single channels
// without any wildcards, but you can subscribe to any of the above
Expand All @@ -152,12 +152,12 @@ mod tests {
fn invalid_incorrect_from_docs() {
// From https://support.pubnub.com/support/solutions/articles/14000043664-how-many-channel-segments-are-supported-with-wildcard-subscribe-

assert_eq!(is_valid("*"), false); // can not wildcard at the top level to subscribe to all channels
assert_eq!(is_valid(".*"), false); // can not start with a .
assert_eq!(is_valid("a.*.b"), false); // * must be at the end
assert_eq!(is_valid("a."), false); // the . must be followed by a * when it is at the end of the name
assert_eq!(is_valid("a*"), false); // * must always be preceded with a .
assert_eq!(is_valid("a*b"), false); // * must always be preceded with a . and .* must always be at the end
assert!(!is_valid("*")); // can not wildcard at the top level to subscribe to all channels
assert!(!is_valid(".*")); // can not start with a .
assert!(!is_valid("a.*.b")); // * must be at the end
assert!(!is_valid("a.")); // the . must be followed by a * when it is at the end of the name
assert!(!is_valid("a*")); // * must always be preceded with a .
assert!(!is_valid("a*b")); // * must always be preceded with a . and .* must always be at the end

// NOTE: The above invalid channel names will actually succeed if you
// attempt to subscribe to them. They will even succeed when you publish
Expand All @@ -182,8 +182,8 @@ mod tests {
// two . characters (more than three segments) it will succeed, but
// you will not be able to publish to those channels.

assert_eq!(is_valid("a.b.c.d"), false); // too many segments
assert_eq!(is_valid("a.b.c.*"), false); // too many segments
assert!(!is_valid("a.b.c.d")); // too many segments
assert!(!is_valid("a.b.c.*")); // too many segments

// If you do attempt to publish to channel names with more than three
// segments (three or more . delimiters), then you will receive a 400
Expand Down
12 changes: 1 addition & 11 deletions pubnub-core/src/data/timetoken.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::time::{SystemTime, SystemTimeError};
/// This is the timetoken structure that PubNub uses as a stream index.
/// It allows clients to resume streaming from where they left off for added
/// resiliency.
#[derive(Debug, Clone, PartialEq, Eq, Hash, Copy)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, Copy, Default)]
pub struct Timetoken {
/// Timetoken
pub t: u64,
Expand Down Expand Up @@ -57,16 +57,6 @@ impl Timetoken {
}
}

impl Default for Timetoken {
#[must_use]
fn default() -> Self {
Self {
t: u64::default(),
r: u32::default(),
}
}
}

impl std::fmt::Display for Timetoken {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
write!(fmt, "{{ t: {}, r: {} }}", self.t, self.r)
Expand Down
4 changes: 4 additions & 0 deletions pubnub-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
//!
//! Build your own client, or use preconfigured [`pubnub-hyper`](pubnub-hyper).
// TODO: Remove these when minimum Rust version >1.59.0, when the name changed.
// TODO: `broken_intra_doc_links` below should become `rustdoc::broken_intra_doc_links`.
#![allow(unknown_lints)]
#![allow(renamed_and_removed_lints)]
#![deny(
clippy::all,
clippy::pedantic,
Expand Down
2 changes: 1 addition & 1 deletion pubnub-core/src/mock/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ impl Runtime for MockRuntime {
where
F: Future<Output = ()> + Send + 'static,
{
self.mock_workaround_spawn(Box::pin(future))
self.mock_workaround_spawn(Box::pin(future));
}
}
6 changes: 3 additions & 3 deletions pubnub-core/src/pubnub/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn mocked_pubnub_publish_ok() {
.expect("unexpected failure");
assert_eq!(timetoken.t, 123);
assert_eq!(timetoken.r, 456);
})
});
}

#[test]
Expand Down Expand Up @@ -185,7 +185,7 @@ fn mocked_pubnub_subscribe_ok() {
})
.unwrap();

pool.run()
pool.run();
}

#[allow(clippy::too_many_lines)]
Expand Down Expand Up @@ -333,5 +333,5 @@ fn mocked_pubnub_subscribe_trasport_error_does_not_stall_loop() {
})
.unwrap();

pool.run()
pool.run();
}
2 changes: 1 addition & 1 deletion pubnub-core/src/subscription/message_destinations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ mod tests {
}

fn assert_iter_eq(m: &Message, expected: &[pubsub::SubscribeTo]) {
let iter = MessageDestinations::new(&m);
let iter = MessageDestinations::new(m);
let vec: Vec<_> = iter.collect();
assert_eq!(vec, expected);
}
Expand Down
1 change: 1 addition & 0 deletions pubnub-core/src/subscription/subscribe_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ enum ControlOutcome {
}

/// Handle a control command.
#[allow(clippy::unused_async)]
async fn handle_control_command(
state_data: &mut StateData,
msg: Option<ControlCommand>,
Expand Down
19 changes: 12 additions & 7 deletions pubnub-core/src/subscription/subscription.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@ use std::pin::Pin;
/// [`PubNub::subscribe`]: crate::pubnub::PubNub::subscribe
#[derive(Debug)]
pub struct Subscription<TRuntime: Runtime> {
pub(crate) runtime: TRuntime, // Runtime to use for managing resources
pub(crate) destination: pubsub::SubscribeTo, // Subscription destination
pub(crate) id: SubscriptionID, // Unique identifier for the listener
pub(crate) control_tx: ControlTx, // For cleaning up resources at the subscribe loop when dropped
pub(crate) runtime: TRuntime,
// Runtime to use for managing resources
pub(crate) destination: pubsub::SubscribeTo,
// Subscription destination
pub(crate) id: SubscriptionID,
// Unique identifier for the listener
pub(crate) control_tx: ControlTx,
// For cleaning up resources at the subscribe loop when dropped
pub(crate) channel_rx: ChannelRx, // Stream that produces messages
}

Expand Down Expand Up @@ -55,9 +59,10 @@ impl<TRuntime: Runtime> Drop for Subscription<TRuntime> {
// See: https://boats.gitlab.io/blog/post/poll-drop/
self.runtime.spawn(async move {
let drop_send_result = control_tx.send(command).await;
if is_drop_send_result_error(drop_send_result) {
panic!("Unable to unsubscribe");
}
assert!(
!is_drop_send_result_error(drop_send_result),
"Unable to unsubscribe"
);
});
}
}
Expand Down
4 changes: 4 additions & 0 deletions pubnub-hyper/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@
//! # };
//! ```
// TODO: Remove these when minimum Rust version >1.59.0, when the name changed.
// TODO: `broken_intra_doc_links` below should become `rustdoc::broken_intra_doc_links`.
#![allow(unknown_lints)]
#![allow(renamed_and_removed_lints)]
#![deny(
clippy::all,
clippy::pedantic,
Expand Down
8 changes: 4 additions & 4 deletions pubnub-hyper/src/transport/hyper/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl TransportService<request::GetHistory> for Hyper {
.set_optional_scalar("end", end)
.set_optional_scalar("include_meta", include_metadata)
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down Expand Up @@ -85,7 +85,7 @@ impl TransportService<request::DeleteHistory> for Hyper {
.set_optional_scalar("start", start)
.set_optional_scalar("end", end)
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Prepare the request.
let req = Request::builder()
Expand Down Expand Up @@ -122,7 +122,7 @@ impl TransportService<request::MessageCountsWithTimetoken> for Hyper {
.set_list("channels", channels)
.set_scalar("timetoken", timetoken)
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down Expand Up @@ -157,7 +157,7 @@ impl TransportService<request::MessageCountsWithChannelTimetokens> for Hyper {
.set_list("channels", names)
.set_list("channelsTimetoken", timetokens)
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down
1 change: 1 addition & 0 deletions pubnub-hyper/src/transport/hyper/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ impl HyperBuilder {
.build::<_, Body>(https)
}

// TODO: Remove random default UUID and require user to specify one. https://github.com/pubnub/rust/issues/66
fn default_uuid() -> UUID {
UUID::random()
}
Expand Down
2 changes: 1 addition & 1 deletion pubnub-hyper/src/transport/hyper/pam.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl TransportService<request::Grant> for Hyper {
.set_scalar("signature", signature)
.set_scalar("timestamp", timestamp.to_string())
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Prepare the request.
let req = Request::builder()
Expand Down
20 changes: 10 additions & 10 deletions pubnub-hyper/src/transport/hyper/presence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl TransportService<request::SetState> for Hyper {
.set_scalar("uuid", uuid)
.set_scalar("state", json::stringify(state))
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down Expand Up @@ -174,7 +174,7 @@ impl TransportService<request::GetState> for Hyper {
.set_list_with_if_empty("channel-group", channel_groups, IfEmpty::Skip)
.set_scalar("uuid", uuid)
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down Expand Up @@ -208,7 +208,7 @@ impl TransportService<request::HereNow<presence::respond_with::OccupancyOnly>> f
.set_list_with_if_empty("channel", channels, IfEmpty::Comma)
.set_list_with_if_empty("channel-group", channel_groups, IfEmpty::Skip)
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down Expand Up @@ -244,7 +244,7 @@ impl TransportService<request::HereNow<presence::respond_with::OccupancyAndUUIDs
.set_list_with_if_empty("channel", channels, IfEmpty::Comma)
.set_list_with_if_empty("channel-group", channel_groups, IfEmpty::Skip)
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down Expand Up @@ -281,7 +281,7 @@ impl TransportService<request::HereNow<presence::respond_with::Full>> for Hyper
.set_list_with_if_empty("channel", channels, IfEmpty::Comma)
.set_list_with_if_empty("channel-group", channel_groups, IfEmpty::Skip)
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down Expand Up @@ -310,7 +310,7 @@ impl TransportService<request::GlobalHereNow<presence::respond_with::OccupancyOn
UriTemplate::new("/v2/presence/sub-key/{sub_key}?disable_uuids=1&state=0")
.set_scalar("sub_key", self.subscribe_key.clone())
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down Expand Up @@ -340,7 +340,7 @@ impl TransportService<request::GlobalHereNow<presence::respond_with::OccupancyAn
UriTemplate::new("/v2/presence/sub-key/{sub_key}?disable_uuids=0&state=0")
.set_scalar("sub_key", self.subscribe_key.clone())
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down Expand Up @@ -372,7 +372,7 @@ impl TransportService<request::GlobalHereNow<presence::respond_with::Full>> for
UriTemplate::new("/v2/presence/sub-key/{sub_key}?disable_uuids=0&state=1")
.set_scalar("sub_key", self.subscribe_key.clone())
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand All @@ -398,7 +398,7 @@ impl TransportService<request::WhereNow> for Hyper {
.set_scalar("sub_key", self.subscribe_key.clone())
.set_scalar("uuid", uuid)
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down Expand Up @@ -441,7 +441,7 @@ impl TransportService<request::Heartbeat> for Hyper {
.set_optional_scalar("heartbeat", heartbeat.map(|e|e.to_string()))
.set_scalar("state", json::stringify(state))
.build();
let url = build_uri(&self, &path_and_query)?;
let url = build_uri(self, &path_and_query)?;

// Send network request.
let response = self.http_client.get(url).await?;
Expand Down
Loading

0 comments on commit ef8379e

Please sign in to comment.