Introduces breaking changes

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<u64>,
 }
 
-#[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),
         }
     }
 }

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;

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<Key> for KeyInfo {
     fn from(key: Key) -> Self {
@@ -64,6 +63,7 @@ pub struct PushManager<Co, Cr, S> {
     registration_id: Option<String>,
     store: S,
     update_rate_limiter: PersistedRateLimiter,
+    verify_connection_rate_limiter: PersistedRateLimiter,
 }
 
 impl<Co: Connection, Cr: Cryptography, S: Storage> PushManager<Co, Cr, S> {
@@ -72,6 +72,19 @@ impl<Co: Connection, Cr: Cryptography, S: Storage> PushManager<Co, Cr, S> {
         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<Co: Connection, Cr: Cryptography, S: Storage> PushManager<Co, Cr, S> {
             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<Co: Connection, Cr: Cryptography, S: Storage> PushManager<Co, Cr, S> {
         Ok(())
     }
 
-    pub fn update(&mut self, new_token: &str) -> error::Result<bool> {
+    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<Co: Connection, Cr: Cryptography, S: Storage> PushManager<Co, Cr, S> {
             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<Co: Connection, Cr: Cryptography, S: Storage> PushManager<Co, Cr, S> {
 
         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<Vec<PushSubscriptionChanged>> {
+        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()?;
 

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<Self> {
-        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<Self> {
+        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<bool> {
+    pub fn update(&self, new_token: &str) -> Result<()> {
         self.internal.lock().unwrap().update(new_token)
     }
 

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"
+};

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();