From a6f7483424207a9dc4b25ddb0e7872e362fbbe5a Mon Sep 17 00:00:00 2001 From: sim Date: Mon, 30 Dec 2024 18:11:29 +0000 Subject: [PATCH 1/5] Add urgency support --- .../autoconnect-common/src/protocol.rs | 10 ++++- .../autoconnect-ws-sm/src/identified/mod.rs | 5 ++- .../src/identified/on_client_msg.rs | 45 ++++++++++++++++++- .../src/identified/on_server_notif.rs | 16 ++++++- .../autoconnect-ws-sm/src/unidentified.rs | 3 +- .../src/extractors/notification_headers.rs | 22 ++++++++- autoendpoint/src/routers/common.rs | 1 + autoendpoint/src/routers/webpush.rs | 15 ++++++- autopush-common/src/db/mod.rs | 29 +++++++++++- autopush-common/src/db/models.rs | 4 ++ 10 files changed, 140 insertions(+), 10 deletions(-) diff --git a/autoconnect/autoconnect-common/src/protocol.rs b/autoconnect/autoconnect-common/src/protocol.rs index d02d623da..97156c42b 100644 --- a/autoconnect/autoconnect-common/src/protocol.rs +++ b/autoconnect/autoconnect-common/src/protocol.rs @@ -12,7 +12,7 @@ use std::str::FromStr; use serde_derive::{Deserialize, Serialize}; use uuid::Uuid; -use autopush_common::notification::Notification; +use autopush_common::{db::Urgency, notification::Notification}; #[derive(Debug, Eq, PartialEq, Serialize)] #[serde(untagged)] @@ -66,6 +66,10 @@ pub enum ClientMessage { version: String, }, + Urgency { + min: Urgency, + }, + Ping, } @@ -123,6 +127,10 @@ pub enum ServerMessage { Notification(Notification), + Urgency { + status: u32, + }, + Ping, } diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/mod.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/mod.rs index d7488a248..00cc7844e 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/mod.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/mod.rs @@ -12,7 +12,7 @@ use autoconnect_common::{ use autoconnect_settings::{AppState, Settings}; use autopush_common::{ - db::User, + db::{Urgency, User}, notification::Notification, util::{ms_since_epoch, user_agent::UserAgentInfo}, }; @@ -291,6 +291,8 @@ pub struct ClientFlags { pub old_record_version: bool, /// First time a user has connected "today" pub emit_channel_metrics: bool, + /// Minimum urgency + pub min_urgency: Urgency, } impl Default for ClientFlags { @@ -301,6 +303,7 @@ impl Default for ClientFlags { check_storage: false, old_record_version: false, emit_channel_metrics: false, + min_urgency: Urgency::VeryLow, } } } diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs index ea277cdf1..e66dba6e9 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs @@ -7,7 +7,11 @@ use autoconnect_common::{ broadcast::Broadcast, protocol::{BroadcastValue, ClientAck, ClientMessage, ServerMessage}, }; -use autopush_common::{endpoint::make_endpoint, util::sec_since_epoch}; +use autopush_common::{ + db::Urgency, + endpoint::make_endpoint, + util::{ms_since_epoch, sec_since_epoch}, +}; use super::WebPushClient; use crate::error::{SMError, SMErrorKind}; @@ -38,6 +42,7 @@ impl WebPushClient { self.nack(code); Ok(vec![]) } + ClientMessage::Urgency { min } => Ok(self.change_min_urgency(min).await?), ClientMessage::Ping => Ok(vec![self.ping()?]), } } @@ -330,4 +335,42 @@ impl WebPushClient { Ok(vec![]) } } + + /// Update minimum urgency for the user and the flag + /// + /// If the new urgency is lower than the previous one, + /// We check pending messages, to send messages that were + /// retained because of their urgency + async fn change_min_urgency( + &mut self, + new_min: Urgency, + ) -> Result, SMError> { + // Change the min urgency + self.flags.min_urgency = new_min; + + let status = if let Some(mut user) = self.app_state.db.get_user(&self.uaid).await? { + let current_urgency = user.urgency.unwrap_or(Urgency::VeryLow); + // We update the user + user.urgency = Some(new_min); + user.connected_at = ms_since_epoch(); + + // if new urgency < previous: fetch pending messages + if self.app_state.db.update_user(&mut user).await.is_ok() { + if new_min < current_urgency { + self.ack_state.unacked_stored_highest = None; + self.current_timestamp = None; + let mut res = vec![ServerMessage::Urgency { status: 200 }]; + res.append(&mut self.check_storage().await?); + // We return the Urgency Ack + pending messages + return Ok(res); + } + 200 + } else { + 500 + } + } else { + 404 + }; + Ok(vec![ServerMessage::Urgency { status }]) + } } diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_server_notif.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_server_notif.rs index 7e6b6f62f..d956c9a36 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_server_notif.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_server_notif.rs @@ -6,7 +6,10 @@ use autopush_common::{ }; use super::WebPushClient; -use crate::error::{SMError, SMErrorKind}; +use crate::{ + error::{SMError, SMErrorKind}, + identified::Urgency, +}; impl WebPushClient { /// Handle a `ServerNotification` for this user @@ -119,7 +122,16 @@ impl WebPushClient { let mut expired_topic_sort_keys = vec![]; messages.retain(|msg| { if !msg.expired(now_sec) { - return true; + if let Some(headers) = msg.headers.as_ref() { + return Urgency::from( + headers + .get("urgency") + .and_then(|v| Some(v.as_str())) + .unwrap_or(""), + ) >= self.flags.min_urgency; + } else { + return true; + } } if msg.sortkey_timestamp.is_none() { expired_topic_sort_keys.push(msg.chidmessageid()); diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/unidentified.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/unidentified.rs index 396543068..27edea083 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/unidentified.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/unidentified.rs @@ -9,7 +9,7 @@ use autoconnect_common::{ }; use autoconnect_settings::{AppState, Settings}; use autopush_common::{ - db::{User, USER_RECORD_VERSION}, + db::{Urgency, User, USER_RECORD_VERSION}, util::{ms_since_epoch, ms_utc_midnight}, }; @@ -145,6 +145,7 @@ impl UnidentifiedClient { .record_version .map_or(true, |rec_ver| rec_ver < USER_RECORD_VERSION), emit_channel_metrics: user.connected_at < ms_utc_midnight(), + min_urgency: user.urgency.unwrap_or(Urgency::VeryLow), ..Default::default() }; user.node_id = Some(self.app_state.router_url.to_owned()); diff --git a/autoendpoint/src/extractors/notification_headers.rs b/autoendpoint/src/extractors/notification_headers.rs index 55987a06d..92beb5c27 100644 --- a/autoendpoint/src/extractors/notification_headers.rs +++ b/autoendpoint/src/extractors/notification_headers.rs @@ -7,7 +7,7 @@ use lazy_static::lazy_static; use regex::Regex; use std::cmp::min; use std::collections::HashMap; -use validator::Validate; +use validator::{Validate, ValidationError}; use validator_derive::Validate; lazy_static! { @@ -37,6 +37,9 @@ pub struct NotificationHeaders { )] pub topic: Option, + #[validate(custom(function = "validate_urgency"))] + pub urgency: Option, + // These fields are validated separately, because the validation is complex // and based upon the content encoding pub encoding: Option, @@ -45,10 +48,21 @@ pub struct NotificationHeaders { pub crypto_key: Option, } +fn validate_urgency(value: &str) -> Result<(), ValidationError> { + if ["very-low", "low", "normal", "high"].contains(&value) { + Ok(()) + } else { + Err(ValidationError::new( + "Value not equal to \"very-low\", \"low\", \"normal\" or \"high\"", + )) + } +} + impl From for HashMap { fn from(headers: NotificationHeaders) -> Self { let mut map = HashMap::new(); + map.insert_opt("urgency", headers.urgency); map.insert_opt("encoding", headers.encoding); map.insert_opt("encryption", headers.encryption); map.insert_opt("encryption_key", headers.encryption_key); @@ -73,11 +87,13 @@ impl NotificationHeaders { .map(|ttl| min(ttl, MAX_NOTIFICATION_TTL as i64)) .ok_or(ApiErrorKind::NoTTL)?; let topic = get_owned_header(req, "topic"); + let urgency = get_owned_header(req, "urgency"); let headers = if has_data { NotificationHeaders { ttl, topic, + urgency, encoding: get_owned_header(req, "content-encoding"), encryption: get_owned_header(req, "encryption").map(Self::strip_header), encryption_key: get_owned_header(req, "encryption-key"), @@ -88,6 +104,7 @@ impl NotificationHeaders { NotificationHeaders { ttl, topic, + urgency, encoding: None, encryption: None, encryption_key: None, @@ -359,6 +376,7 @@ mod tests { NotificationHeaders { ttl: 10, topic: None, + urgency: None, encoding: Some("aesgcm".to_string()), encryption: Some("salt=foo".to_string()), encryption_key: None, @@ -384,6 +402,7 @@ mod tests { NotificationHeaders { ttl: 10, topic: None, + urgency: None, encoding: Some("aes128gcm".to_string()), encryption: Some("notsalt=foo".to_string()), encryption_key: None, @@ -410,6 +429,7 @@ mod tests { NotificationHeaders { ttl: 10, topic: None, + urgency: None, encoding: Some("aesgcm".to_string()), encryption: Some("salt=foo".to_string()), encryption_key: None, diff --git a/autoendpoint/src/routers/common.rs b/autoendpoint/src/routers/common.rs index 3a0cf49fd..2aecec680 100644 --- a/autoendpoint/src/routers/common.rs +++ b/autoendpoint/src/routers/common.rs @@ -246,6 +246,7 @@ pub mod tests { }, headers: NotificationHeaders { ttl: 0, + urgency: None, topic: Some("test-topic".to_string()), encoding: Some("test-encoding".to_string()), encryption: Some("test-encryption".to_string()), diff --git a/autoendpoint/src/routers/webpush.rs b/autoendpoint/src/routers/webpush.rs index bdcd915c5..9823aa319 100644 --- a/autoendpoint/src/routers/webpush.rs +++ b/autoendpoint/src/routers/webpush.rs @@ -1,4 +1,5 @@ use async_trait::async_trait; +use autopush_common::db::Urgency; use cadence::{Counted, CountedExt, StatsdClient, Timed}; use reqwest::{Response, StatusCode}; use serde_json::Value; @@ -46,8 +47,20 @@ impl Router for WebPushRouter { ); trace!("✉ Notification = {:?}", notification); + let notif_urgency = ¬ification + .headers + .urgency + .as_ref() + .and_then(|v| Some(Urgency::from(v.as_str()))); + // If the notification urgency is lower than the user one, we do not send it + if notif_urgency < &user.urgency { + trace!( + "✉ Notification has an urgency lower than the user one: {:?} < {:?}", + ¬if_urgency, + &user.urgency + ); // Check if there is a node connected to the client - if let Some(node_id) = &user.node_id { + } else if let Some(node_id) = &user.node_id { trace!( "✉ User has a node ID, sending notification to node: {}", &node_id diff --git a/autopush-common/src/db/mod.rs b/autopush-common/src/db/mod.rs index 70a2f0254..b4cfeb024 100644 --- a/autopush-common/src/db/mod.rs +++ b/autopush-common/src/db/mod.rs @@ -15,8 +15,8 @@ use std::result::Result as StdResult; use derive_builder::Builder; use lazy_static::lazy_static; use regex::RegexSet; -use serde::Serializer; -use serde_derive::{Deserialize, Serialize}; +use serde::{Serialize, Serializer}; +use serde_derive::Deserialize; use uuid::Uuid; #[cfg(feature = "bigtable")] @@ -148,6 +148,9 @@ pub struct User { /// Last node/port the client was or may be connected to #[serde(skip_serializing_if = "Option::is_none")] pub node_id: Option, + /// Last minimum urgency set by the client + #[serde(skip_serializing_if = "Option::is_none")] + pub urgency: Option, /// Record version #[serde(skip_serializing_if = "Option::is_none")] pub record_version: Option, @@ -184,6 +187,7 @@ impl Default for User { router_type: "webpush".to_string(), router_data: None, node_id: None, + urgency: None, record_version: Some(USER_RECORD_VERSION), current_timestamp: None, version: Some(Uuid::new_v4()), @@ -203,6 +207,27 @@ impl User { } } +#[repr(u8)] +#[derive(Debug, PartialEq, PartialOrd, Serialize, Deserialize, Clone, Copy)] +#[serde(rename_all = "kebab-case")] +pub enum Urgency { + VeryLow = 0, + Low = 1, + Normal = 2, + High = 3, +} + +impl From<&str> for Urgency { + fn from(value: &str) -> Self { + match value.to_lowercase().as_str() { + "high" => Urgency::High, + "low" => Urgency::Low, + "very-low" => Urgency::VeryLow, + _ => Urgency::Normal, + } + } +} + /// A stored Notification record. This is a notification that is to be stored /// until the User Agent reconnects. These are then converted to publishable /// [crate::db::Notification] records. diff --git a/autopush-common/src/db/models.rs b/autopush-common/src/db/models.rs index 0ea633572..47a7d7947 100644 --- a/autopush-common/src/db/models.rs +++ b/autopush-common/src/db/models.rs @@ -19,6 +19,8 @@ pub(crate) struct NotificationHeaders { encryption_key: Option, #[serde(skip_serializing_if = "Option::is_none")] encoding: Option, + #[serde(skip_serializing_if = "Option::is_none")] + urgency: Option, } #[allow(clippy::implicit_hasher)] @@ -29,6 +31,7 @@ impl From for HashMap { map.insert_opt("encryption", val.encryption); map.insert_opt("encryption_key", val.encryption_key); map.insert_opt("encoding", val.encoding); + map.insert_opt("urgency", val.urgency); map } } @@ -40,6 +43,7 @@ impl From> for NotificationHeaders { encryption: val.get("encryption").map(|v| v.to_string()), encryption_key: val.get("encryption_key").map(|v| v.to_string()), encoding: val.get("encoding").map(|v| v.to_string()), + urgency: val.get("urgency").map(|v| v.to_string()), } } } From 960117844df722be10bf8bb3f1db3fce2a9fae44 Mon Sep 17 00:00:00 2001 From: sim Date: Fri, 3 Jan 2025 07:27:29 +0000 Subject: [PATCH 2/5] Impl From> for Urgency --- .../autoconnect-ws-sm/src/identified/on_server_notif.rs | 7 +------ autoendpoint/src/routers/webpush.rs | 8 ++------ autopush-common/src/db/mod.rs | 6 ++++++ 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_server_notif.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_server_notif.rs index d956c9a36..ff64b64cb 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_server_notif.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_server_notif.rs @@ -123,12 +123,7 @@ impl WebPushClient { messages.retain(|msg| { if !msg.expired(now_sec) { if let Some(headers) = msg.headers.as_ref() { - return Urgency::from( - headers - .get("urgency") - .and_then(|v| Some(v.as_str())) - .unwrap_or(""), - ) >= self.flags.min_urgency; + return Urgency::from(headers.get("urgency")) >= self.flags.min_urgency; } else { return true; } diff --git a/autoendpoint/src/routers/webpush.rs b/autoendpoint/src/routers/webpush.rs index 9823aa319..58792f2f5 100644 --- a/autoendpoint/src/routers/webpush.rs +++ b/autoendpoint/src/routers/webpush.rs @@ -47,13 +47,9 @@ impl Router for WebPushRouter { ); trace!("✉ Notification = {:?}", notification); - let notif_urgency = ¬ification - .headers - .urgency - .as_ref() - .and_then(|v| Some(Urgency::from(v.as_str()))); + let notif_urgency = Urgency::from(notification.headers.urgency.as_ref()); // If the notification urgency is lower than the user one, we do not send it - if notif_urgency < &user.urgency { + if notif_urgency < user.urgency.unwrap_or(Urgency::VeryLow) { trace!( "✉ Notification has an urgency lower than the user one: {:?} < {:?}", ¬if_urgency, diff --git a/autopush-common/src/db/mod.rs b/autopush-common/src/db/mod.rs index b4cfeb024..0f7e5e830 100644 --- a/autopush-common/src/db/mod.rs +++ b/autopush-common/src/db/mod.rs @@ -228,6 +228,12 @@ impl From<&str> for Urgency { } } +impl From> for Urgency { + fn from(value: Option<&String>) -> Self { + Urgency::from(value.and_then(|v| Some(v.as_str())).unwrap_or("")) + } +} + /// A stored Notification record. This is a notification that is to be stored /// until the User Agent reconnects. These are then converted to publishable /// [crate::db::Notification] records. From af005464a89e8e14f07a3ebd20b06b7d122669a0 Mon Sep 17 00:00:00 2001 From: sim Date: Fri, 3 Jan 2025 07:33:23 +0000 Subject: [PATCH 3/5] Add comments when falling back to default urgency filter --- .../autoconnect-ws-sm/src/identified/on_client_msg.rs | 2 ++ autoendpoint/src/routers/webpush.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs index e66dba6e9..a05292447 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs @@ -349,6 +349,8 @@ impl WebPushClient { self.flags.min_urgency = new_min; let status = if let Some(mut user) = self.app_state.db.get_user(&self.uaid).await? { + // If the user hasn't set a minimum urgency yet, they receive all messages, + // which is equivalent to setting very-low as a minimum let current_urgency = user.urgency.unwrap_or(Urgency::VeryLow); // We update the user user.urgency = Some(new_min); diff --git a/autoendpoint/src/routers/webpush.rs b/autoendpoint/src/routers/webpush.rs index 58792f2f5..2df52fcb3 100644 --- a/autoendpoint/src/routers/webpush.rs +++ b/autoendpoint/src/routers/webpush.rs @@ -49,6 +49,7 @@ impl Router for WebPushRouter { let notif_urgency = Urgency::from(notification.headers.urgency.as_ref()); // If the notification urgency is lower than the user one, we do not send it + // If the user hasn't set a minimum urgency, we accept all notifications if notif_urgency < user.urgency.unwrap_or(Urgency::VeryLow) { trace!( "✉ Notification has an urgency lower than the user one: {:?} < {:?}", From f39fce0f98ca74a2d863df67581320a90b91443e Mon Sep 17 00:00:00 2001 From: sim Date: Fri, 3 Jan 2025 07:58:06 +0000 Subject: [PATCH 4/5] Add urgency feature --- autoconnect/Cargo.toml | 1 + autoconnect/autoconnect-common/Cargo.toml | 1 + autoconnect/autoconnect-common/src/protocol.rs | 7 ++++++- autoconnect/autoconnect-ws/Cargo.toml | 1 + .../autoconnect-ws/autoconnect-ws-sm/Cargo.toml | 1 + .../autoconnect-ws-sm/src/identified/on_client_msg.rs | 11 ++++++----- 6 files changed, 16 insertions(+), 6 deletions(-) diff --git a/autoconnect/Cargo.toml b/autoconnect/Cargo.toml index c0ba4ba27..6b6a4087b 100644 --- a/autoconnect/Cargo.toml +++ b/autoconnect/Cargo.toml @@ -56,4 +56,5 @@ docopt = "1.1" default = ["bigtable"] bigtable = ["autopush_common/bigtable", "autoconnect_settings/bigtable"] emulator = ["bigtable"] +urgency = ["autoconnect_ws/urgency"] log_vapid = [] diff --git a/autoconnect/autoconnect-common/Cargo.toml b/autoconnect/autoconnect-common/Cargo.toml index 8c3eaba1c..b458485a6 100644 --- a/autoconnect/autoconnect-common/Cargo.toml +++ b/autoconnect/autoconnect-common/Cargo.toml @@ -27,3 +27,4 @@ autopush_common.workspace = true [features] test-support = [] +urgency = [] diff --git a/autoconnect/autoconnect-common/src/protocol.rs b/autoconnect/autoconnect-common/src/protocol.rs index 97156c42b..a8c44bb42 100644 --- a/autoconnect/autoconnect-common/src/protocol.rs +++ b/autoconnect/autoconnect-common/src/protocol.rs @@ -12,7 +12,10 @@ use std::str::FromStr; use serde_derive::{Deserialize, Serialize}; use uuid::Uuid; -use autopush_common::{db::Urgency, notification::Notification}; +use autopush_common::notification::Notification; + +#[cfg(feature = "urgency")] +use autopush_common::db::Urgency; #[derive(Debug, Eq, PartialEq, Serialize)] #[serde(untagged)] @@ -66,6 +69,7 @@ pub enum ClientMessage { version: String, }, + #[cfg(feature = "urgency")] Urgency { min: Urgency, }, @@ -127,6 +131,7 @@ pub enum ServerMessage { Notification(Notification), + #[cfg(feature = "urgency")] Urgency { status: u32, }, diff --git a/autoconnect/autoconnect-ws/Cargo.toml b/autoconnect/autoconnect-ws/Cargo.toml index 194d79742..ff1dbfd7d 100644 --- a/autoconnect/autoconnect-ws/Cargo.toml +++ b/autoconnect/autoconnect-ws/Cargo.toml @@ -35,3 +35,4 @@ ctor.workspace = true autoconnect_common = { workspace = true, features = ["test-support"] } [features] +urgency = ["autoconnect_ws_sm/urgency"] diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/Cargo.toml b/autoconnect/autoconnect-ws/autoconnect-ws-sm/Cargo.toml index bc76baf62..89b95ca2f 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/Cargo.toml +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/Cargo.toml @@ -32,3 +32,4 @@ serde_json.workspace = true autoconnect_common = { workspace = true, features = ["test-support"] } [features] +urgency = ["autoconnect_common/urgency"] diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs index a05292447..0a0f3d3d7 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs @@ -7,11 +7,10 @@ use autoconnect_common::{ broadcast::Broadcast, protocol::{BroadcastValue, ClientAck, ClientMessage, ServerMessage}, }; -use autopush_common::{ - db::Urgency, - endpoint::make_endpoint, - util::{ms_since_epoch, sec_since_epoch}, -}; +use autopush_common::{endpoint::make_endpoint, util::sec_since_epoch}; + +#[cfg(feature = "urgency")] +use autopush_common::{db::Urgency, util::ms_since_epoch}; use super::WebPushClient; use crate::error::{SMError, SMErrorKind}; @@ -42,6 +41,7 @@ impl WebPushClient { self.nack(code); Ok(vec![]) } + #[cfg(feature = "urgency")] ClientMessage::Urgency { min } => Ok(self.change_min_urgency(min).await?), ClientMessage::Ping => Ok(vec![self.ping()?]), } @@ -341,6 +341,7 @@ impl WebPushClient { /// If the new urgency is lower than the previous one, /// We check pending messages, to send messages that were /// retained because of their urgency + #[cfg(feature = "urgency")] async fn change_min_urgency( &mut self, new_min: Urgency, From fa187a31605d36d36ffbe5cc25f3146252fe6f77 Mon Sep 17 00:00:00 2001 From: sim Date: Fri, 10 Jan 2025 09:12:08 +0100 Subject: [PATCH 5/5] Correctly handle update_user error for min urgency --- .../src/identified/on_client_msg.rs | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs index 0a0f3d3d7..79b1ba74d 100644 --- a/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs +++ b/autoconnect/autoconnect-ws/autoconnect-ws-sm/src/identified/on_client_msg.rs @@ -349,31 +349,26 @@ impl WebPushClient { // Change the min urgency self.flags.min_urgency = new_min; - let status = if let Some(mut user) = self.app_state.db.get_user(&self.uaid).await? { + if let Some(mut user) = self.app_state.db.get_user(&self.uaid).await? { // If the user hasn't set a minimum urgency yet, they receive all messages, // which is equivalent to setting very-low as a minimum let current_urgency = user.urgency.unwrap_or(Urgency::VeryLow); + // We update the user user.urgency = Some(new_min); user.connected_at = ms_since_epoch(); + self.app_state.db.update_user(&mut user).await?; + let mut res = vec![ServerMessage::Urgency { status: 200 }]; // if new urgency < previous: fetch pending messages - if self.app_state.db.update_user(&mut user).await.is_ok() { - if new_min < current_urgency { - self.ack_state.unacked_stored_highest = None; - self.current_timestamp = None; - let mut res = vec![ServerMessage::Urgency { status: 200 }]; - res.append(&mut self.check_storage().await?); - // We return the Urgency Ack + pending messages - return Ok(res); - } - 200 - } else { - 500 + if new_min < current_urgency { + self.ack_state.unacked_stored_highest = None; + self.current_timestamp = None; + res.append(&mut self.check_storage().await?); } + Ok(res) } else { - 404 - }; - Ok(vec![ServerMessage::Urgency { status }]) + Ok(vec![ServerMessage::Urgency { status: 404 }]) + } } }