f reviews

autoconnect/autoconnect-common/src/test_support.rs

@@ -15,7 +15,7 @@ pub const HELLO: &str = r#"{"messageType": "hello", "use_webpush": true}"#;
 pub const HELLO_AGAIN: &str = r#"{"messageType": "hello", "use_webpush": true,
                                   "uaid": "deadbeef-0000-0000-deca-fbad00000000"}"#;
 
-pub const CURRENT_MONTH: &str = "message_2023_5";
+pub const CURRENT_MONTH: &str = "message_2018_06";
 
 /// Return a simple MockDbClient that responds to hello (once) with a new uaid.
 pub fn hello_db() -> MockDbClient {

autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/mod.rs

@@ -119,10 +119,7 @@ impl WebPushClient {
     /// Cleanup after the session has ended
     pub fn shutdown(&mut self, reason: Option<String>) {
         trace!("WebPushClient::shutdown");
-        // Save any unAck'd Direct notifs
-        if !self.ack_state.unacked_direct_notifs.is_empty() {
-            self.save_and_notify_unacked_direct_notifs();
-        }
+        self.save_and_notify_unacked_direct_notifs();
 
         let ua_info = &self.ua_info;
         let stats = &self.stats;
@@ -163,8 +160,15 @@ impl WebPushClient {
     /// Direct messages are solely stored in memory until Ack'd by the Client,
     /// so on shutdown, any not Ack'd are stored in the db to not be lost
     fn save_and_notify_unacked_direct_notifs(&mut self) {
-        trace!("WebPushClient::save_and_notify_unacked_direct_notifs");
         let mut notifs = mem::take(&mut self.ack_state.unacked_direct_notifs);
+        trace!(
+            "WebPushClient::save_and_notify_unacked_direct_notifs len: {}",
+            notifs.len()
+        );
+        if notifs.is_empty() {
+            return;
+        }
+
         self.stats.direct_storage += notifs.len() as i32;
         // TODO: clarify this comment re the Python version
         // Ensure we don't store these as legacy by setting a 0 as the

autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs

@@ -278,6 +278,7 @@ impl WebPushClient {
         debug_assert!(!self.ack_state.unacked_notifs());
         let flags = &self.flags;
         if flags.rotate_message_table {
+            // TODO: probably remove entirely
             debug!("▶️ WebPushClient:post_process_all_acked rotate_message_table");
             unimplemented!()
         } else if flags.reset_uaid {

autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_server_notif.rs

@@ -156,64 +156,72 @@ impl WebPushClient {
     }
 
     /// Read a chunk (max count 10 returned) of Notifications from storage
+    ///
+    /// TODO: document topic vs timestamp messages
     async fn do_check_storage(&self) -> Result<CheckStorageResponse, SMError> {
         trace!("🗄️ WebPushClient::do_check_storage");
         let timestamp = self.ack_state.unacked_stored_highest;
-        let resp = if self.flags.include_topic {
+        let topic_resp = if self.flags.include_topic {
             trace!("🗄️ WebPushClient::do_check_storage: fetch_messages");
             self.app_state.db.fetch_messages(&self.uaid, 11).await?
         } else {
             Default::default()
         };
-        if !resp.messages.is_empty() {
+        if !topic_resp.messages.is_empty() {
             trace!(
                 "🗄️ WebPushClient::do_check_storage: Topic message returns: {:#?}",
-                resp.messages
+                topic_resp.messages
             );
             self.app_state
                 .metrics
-                .count_with_tags("notification.message.retrieved", resp.messages.len() as i64)
+                .count_with_tags(
+                    "notification.message.retrieved",
+                    topic_resp.messages.len() as i64,
+                )
                 .with_tag("topic", "true")
                 .send();
             return Ok(CheckStorageResponse {
                 include_topic: true,
-                messages: resp.messages,
-                timestamp: resp.timestamp,
+                messages: topic_resp.messages,
+                timestamp: topic_resp.timestamp,
             });
         }
 
         let timestamp = if self.flags.include_topic {
-            resp.timestamp
+            topic_resp.timestamp
         } else {
             timestamp
         };
         trace!(
             "🗄️ WebPushClient::do_check_storage: fetch_timestamp_messages timestamp: {:?}",
             timestamp
         );
-        let resp = self
+        let timestamp_resp = self
             .app_state
             .db
             .fetch_timestamp_messages(&self.uaid, timestamp, 10)
             .await?;
-        if !resp.messages.is_empty() {
+        if !timestamp_resp.messages.is_empty() {
             trace!(
                 "🗄️ WebPushClient::do_check_storage: Timestamp message returns: {:#?}",
-                resp.messages
+                timestamp_resp.messages
             );
             self.app_state
                 .metrics
-                .count_with_tags("notification.message.retrieved", resp.messages.len() as i64)
+                .count_with_tags(
+                    "notification.message.retrieved",
+                    timestamp_resp.messages.len() as i64,
+                )
                 .with_tag("topic", "false")
                 .send();
         }
 
         Ok(CheckStorageResponse {
             include_topic: false,
-            messages: resp.messages,
+            messages: timestamp_resp.messages,
             // If we didn't get a timestamp off the last query, use the
             // original value if passed one
-            timestamp: resp.timestamp.or(timestamp),
+            timestamp: timestamp_resp.timestamp.or(timestamp),
         })
     }