From a34eafae65c98805c59cdfa7364575bb1a36270e Mon Sep 17 00:00:00 2001 From: Tarik Eshaq Date: Thu, 20 Apr 2023 16:09:37 -0400 Subject: [PATCH] Introduces breaking changes --- components/push/src/internal/config.rs | 17 ++++--- components/push/src/internal/mod.rs | 1 - components/push/src/internal/push_manager.rs | 39 ++++++++++------ components/push/src/lib.rs | 48 ++++---------------- components/push/src/push.udl | 25 +++++++--- examples/push-livetest/src/livetest.rs | 22 +++++---- 6 files changed, 76 insertions(+), 76 deletions(-) diff --git a/components/push/src/internal/config.rs b/components/push/src/internal/config.rs index 0f91a39e0..721191e77 100644 --- a/components/push/src/internal/config.rs +++ b/components/push/src/internal/config.rs @@ -7,6 +7,8 @@ use std::{fmt::Display, str::FromStr}; +pub const DEFAULT_VERIFY_CONNECTION_LIMITER_INTERVAL: u64 = 24 * 60 * 60; // 24 hours. + use crate::PushError; /// The types of supported native bridges. /// @@ -60,10 +62,16 @@ pub struct PushConfiguration { /// OS Path to the database pub database_path: String, + + /// Number of seconds between to rate limit + /// the verify connection call + /// defaults to 24 hours + pub verify_connection_rate_limiter: Option, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default)] pub enum Protocol { + #[default] Https, Http, } @@ -81,12 +89,6 @@ impl Display for Protocol { } } -impl Default for Protocol { - fn default() -> Self { - Protocol::Https - } -} - impl FromStr for Protocol { type Err = PushError; @@ -108,6 +110,7 @@ impl Default for PushConfiguration { bridge_type: Default::default(), sender_id: String::from(""), database_path: String::from(""), + verify_connection_rate_limiter: Some(DEFAULT_VERIFY_CONNECTION_LIMITER_INTERVAL), } } } diff --git a/components/push/src/internal/mod.rs b/components/push/src/internal/mod.rs index 63615c5cf..59df1f80d 100644 --- a/components/push/src/internal/mod.rs +++ b/components/push/src/internal/mod.rs @@ -8,5 +8,4 @@ pub mod crypto; pub mod push_manager; pub mod storage; -pub(crate) use config::PushConfiguration; pub(crate) use push_manager::PushManager; diff --git a/components/push/src/internal/push_manager.rs b/components/push/src/internal/push_manager.rs index f00b9f02a..e6e7954ee 100644 --- a/components/push/src/internal/push_manager.rs +++ b/components/push/src/internal/push_manager.rs @@ -24,9 +24,8 @@ use crate::{ }; use super::crypto::Cryptography; - -const UPDATE_RATE_LIMITER_INTERVAL: u64 = 24 * 60 * 60; // 500 calls per 24 hours. -const UPDATE_RATE_LIMITER_MAX_CALLS: u16 = 500; +const UPDATE_RATE_LIMITER_INTERVAL: u64 = 24 * 60 * 60; // 24 hours. +const UPDATE_RATE_LIMITER_MAX_CALLS: u16 = 500; // 500 impl From for KeyInfo { fn from(key: Key) -> Self { @@ -64,6 +63,7 @@ pub struct PushManager { registration_id: Option, store: S, update_rate_limiter: PersistedRateLimiter, + verify_connection_rate_limiter: PersistedRateLimiter, } impl PushManager { @@ -72,6 +72,19 @@ impl PushManager { let uaid = store.get_uaid()?; let auth = store.get_auth()?; let registration_id = store.get_registration_id()?; + let verify_connection_rate_limiter = PersistedRateLimiter::new( + "verify_connection", + config + .verify_connection_rate_limiter + .unwrap_or(super::config::DEFAULT_VERIFY_CONNECTION_LIMITER_INTERVAL), + 1, + ); + + let update_rate_limiter = PersistedRateLimiter::new( + "update_token", + UPDATE_RATE_LIMITER_INTERVAL, + UPDATE_RATE_LIMITER_MAX_CALLS, + ); Ok(Self { connection: Co::connect(config), @@ -80,11 +93,8 @@ impl PushManager { auth, registration_id, store, - update_rate_limiter: PersistedRateLimiter::new( - "update_token", - UPDATE_RATE_LIMITER_INTERVAL, - UPDATE_RATE_LIMITER_MAX_CALLS, - ), + update_rate_limiter, + verify_connection_rate_limiter, }) } @@ -168,13 +178,13 @@ impl PushManager { Ok(()) } - pub fn update(&mut self, new_token: &str) -> error::Result { + pub fn update(&mut self, new_token: &str) -> error::Result<()> { if self.registration_id.as_deref() == Some(new_token) { // Already up to date! // if we haven't send it to the server yet, we will on the next subscribe! // if we have sent it to the server, no need to do so again. We will catch any issues // through the [`PushManager::verify_connection`] check - return Ok(false); + return Ok(()); } // It's OK if we don't have a uaid yet - that means we don't have any subscriptions, @@ -185,11 +195,11 @@ impl PushManager { log::info!( "saved the registration ID but not telling the server as we have no subs yet" ); - return Ok(false); + return Ok(()); } if !self.update_rate_limiter.check(&self.store) { - return Ok(true); + return Ok(()); } let (uaid, auth) = self.ensure_auth_pair()?; @@ -207,10 +217,13 @@ impl PushManager { self.store.set_registration_id(new_token)?; self.registration_id = Some(new_token.to_string()); - Ok(true) + Ok(()) } pub fn verify_connection(&mut self) -> Result> { + if !self.verify_connection_rate_limiter.check(&self.store) { + return Ok(vec![]); + } let channels = self.store.get_channel_list()?; let (uaid, auth) = self.ensure_auth_pair()?; diff --git a/components/push/src/lib.rs b/components/push/src/lib.rs index bf23316fe..ac038f246 100644 --- a/components/push/src/lib.rs +++ b/components/push/src/lib.rs @@ -183,15 +183,13 @@ uniffi::include_scaffolding!("push"); // All implementation detail lives in the `internal` module mod internal; -use std::str::FromStr; use std::sync::Mutex; mod error; use error::*; use internal::communications::ConnectHttp; -pub use internal::config::BridgeType; +pub use internal::config::{BridgeType, Protocol as PushHttpProtocol, PushConfiguration}; use internal::crypto::Crypto; -use internal::PushConfiguration; pub use error::PushError; use internal::storage::Store; @@ -215,40 +213,18 @@ impl PushManager { /// channels /// /// # Arguments - /// - /// - `sender_id` - Sender/Application ID value - /// - `server_host` - The host name for the service (e.g. "updates.push.services.mozilla.com"). - /// - `http_protocol` - The optional socket protocol (default: "https") - /// - `bridge_type` - The [`BridgeType`] the consumer would like to use to deliver the push messages - /// - `registration_id` - The native OS messaging registration ID - /// - `database_path` - The path where [`PushManager`] will store persisted state + /// - `config`: [`PushConfiguration`] the configuration for this instance of PushManager /// /// # Errors /// Returns an error in the following cases: /// - PushManager is unable to open the `database_path` given /// - PushManager is unable to establish a connection to the autopush server - pub fn new( - sender_id: String, - server_host: String, - http_protocol: String, - bridge_type: BridgeType, - registration_id: String, - database_path: String, - ) -> Result { - log::debug!("PushManager server_host: {server_host}, http_protocol: {http_protocol}"); - if !registration_id.is_empty() { - log::warn!("`registration_id` is ignored/deprecated when creating a push manager."); - } - // XXX - we probably should persist, say, this as JSON and ensure it's the same - // on each run, then nuke the DB if not. Eg, imagine "bridge_type" changing, things - // would break badly. Unlikely, so later... - let config = PushConfiguration { - server_host, - http_protocol: internal::config::Protocol::from_str(&http_protocol)?, - bridge_type, - sender_id, - database_path, - }; + pub fn new(config: PushConfiguration) -> Result { + log::debug!( + "PushManager server_host: {}, http_protocol: {}", + config.server_host, + config.http_protocol + ); Ok(Self { internal: Mutex::new(internal::PushManager::new(config)?), }) @@ -313,21 +289,15 @@ impl PushManager { } /// Updates the Native OS push registration ID. - /// **NOTE**: If this returns false, the subsequent [`PushManager::verify_connection`] - /// may result in new endpoint registration /// /// # Arguments: /// - `new_token` - the new Native OS push registration ID - /// - /// # Returns - /// Returns a boolean indicating if the update was successful - /// /// # Errors /// Return an error in the following cases: /// - The PushManager does not contain a valid UAID /// - An error occurred sending an update request to the autopush server /// - An error occurred accessing the PushManager's persisted storage - pub fn update(&self, new_token: &str) -> Result { + pub fn update(&self, new_token: &str) -> Result<()> { self.internal.lock().unwrap().update(new_token) } diff --git a/components/push/src/push.udl b/components/push/src/push.udl index cfb4bce5d..c0508fa29 100644 --- a/components/push/src/push.udl +++ b/components/push/src/push.udl @@ -20,7 +20,6 @@ interface PushManager { // - `server_host` - The host name for the service (e.g. "updates.push.services.mozilla.com"). // - `http_protocol` - The optional socket protocol (default: "https") // - `bridge_type` - The [`BridgeType`] the consumer would like to use to deliver the push messages - // - `registration_id` - NOT USED. REMOVE ME. // - `database_path` - The path where [`PushManager`] will store persisted state // // # Errors @@ -28,7 +27,7 @@ interface PushManager { // - PushManager is unable to open the `database_path` given // - PushManager is unable to establish a connection to the autopush server [Throws=PushError] - constructor(string sender_id, optional string server_host = "updates.push.services.mozilla.com", optional string http_protocol = "https", BridgeType bridge_type, optional string registration_id = "", optional string database_path = "push.sqlite"); + constructor(PushConfiguration config); // Subscribes to a new channel and gets the Subscription Info block // @@ -89,16 +88,13 @@ interface PushManager { // # Arguments: // - `new_token` - the new Native OS push registration ID // - // # Returns - // Returns a boolean indicating if tried to tell the server about it. - // // # Errors // Return an error in the following cases: // - The PushManager does not contain a valid UAID // - An error occurred sending an update request to the autopush server // - An error occurred accessing the PushManager's persisted storage [Throws=PushError] - boolean update([ByRef] string registration_token); + void update([ByRef] string registration_token); // Verifies the connection state // @@ -255,3 +251,20 @@ enum BridgeType { "Adm", "Apns", }; + +dictionary PushConfiguration { + string server_host; + PushHttpProtocol http_protocol; + BridgeType bridge_type; + string sender_id; + string database_path; + u64? verify_connection_rate_limiter; +}; + +// Supported protocols for push +// "Https" is default, and "Http" is only +// supported for tests +enum PushHttpProtocol { + "Https", + "Http" +}; diff --git a/examples/push-livetest/src/livetest.rs b/examples/push-livetest/src/livetest.rs index de7547fb3..f05917779 100644 --- a/examples/push-livetest/src/livetest.rs +++ b/examples/push-livetest/src/livetest.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use push::{BridgeType, PushManager}; +use push::{BridgeType, PushConfiguration, PushManager}; /** Perform a "Live" test against a locally configured push server * @@ -24,15 +24,17 @@ fn test_live_server() { let tempdir = tempfile::tempdir().unwrap(); viaduct_reqwest::use_reqwest_backend(); - let pm = PushManager::new( - "fir-bridgetest".to_string(), - "localhost:8082".to_string(), - "http".to_string(), - BridgeType::Fcm, - "".to_string(), - tempdir.path().join("test.db").to_string_lossy().to_string(), - ) - .unwrap(); + let push_config = PushConfiguration { + server_host: "localhost:8082".to_string(), + + http_protocol: push::PushHttpProtocol::Http, + bridge_type: BridgeType::Fcm, + sender_id: "".to_string(), + database_path: tempdir.path().join("test.db").to_string_lossy().to_string(), + verify_connection_rate_limiter: Some(0), + }; + + let pm = PushManager::new(push_config).unwrap(); let channel1 = dummy_uuid(); let channel2 = dummy_uuid();