-
Notifications
You must be signed in to change notification settings - Fork 21
Feat: Add urgency support #841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 5 commits
bf6c2cc
311d47d
246b415
bf66106
eb5a95d
149f450
67c4b83
95d9a99
b16adc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -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}, | ||||||||
| }; | ||||||||
|
|
@@ -309,6 +309,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, | ||||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| } | ||||||||
|
|
||||||||
| impl Default for ClientFlags { | ||||||||
|
|
@@ -319,6 +321,7 @@ impl Default for ClientFlags { | |||||||
| check_storage: false, | ||||||||
| old_record_version: false, | ||||||||
| emit_channel_metrics: false, | ||||||||
| min_urgency: Urgency::VeryLow, | ||||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,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 | ||
|
|
@@ -123,7 +126,11 @@ impl WebPushClient { | |
| // inner msg.clone() | ||
| messages.retain(|msg| { | ||
| if !msg.expired(now_sec) { | ||
| return true; | ||
| if let Some(headers) = msg.headers.as_ref() { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| return Urgency::from(headers.get("urgency")) >= self.flags.min_urgency; | ||
| } else { | ||
| return true; | ||
| } | ||
| } | ||
| if msg.sortkey_timestamp.is_none() { | ||
| expired_messages.push(msg.clone()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -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 | ||||||||
| .is_none_or(|rec_ver| rec_ver < USER_RECORD_VERSION), | ||||||||
| emit_channel_metrics: user.connected_at < ms_utc_midnight(), | ||||||||
| min_urgency: user.urgency.unwrap_or(Urgency::VeryLow), | ||||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| ..Default::default() | ||||||||
| }; | ||||||||
| user.node_id = Some(self.app_state.router_url.to_owned()); | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<String>, | ||
|
|
||
| #[validate(custom(function = "validate_urgency"))] | ||
| pub urgency: Option<String>, | ||
|
|
||
| // These fields are validated separately, because the validation is complex | ||
| // and based upon the content encoding | ||
| pub encoding: Option<String>, | ||
|
|
@@ -45,10 +48,21 @@ pub struct NotificationHeaders { | |
| pub crypto_key: Option<String>, | ||
| } | ||
|
|
||
| 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<NotificationHeaders> for HashMap<String, String> { | ||
| 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.num_seconds())) | ||
| .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, | ||
|
|
@@ -365,6 +382,7 @@ mod tests { | |
| NotificationHeaders { | ||
| ttl: 10, | ||
| topic: None, | ||
| urgency: None, | ||
| encoding: Some("aesgcm".to_string()), | ||
| encryption: Some("salt=foo".to_string()), | ||
| encryption_key: None, | ||
|
|
@@ -390,6 +408,7 @@ mod tests { | |
| NotificationHeaders { | ||
| ttl: 10, | ||
| topic: None, | ||
| urgency: None, | ||
| encoding: Some("aes128gcm".to_string()), | ||
| encryption: Some("notsalt=foo".to_string()), | ||
| encryption_key: None, | ||
|
|
@@ -416,6 +435,7 @@ mod tests { | |
| NotificationHeaders { | ||
| ttl: 10, | ||
| topic: None, | ||
| urgency: None, | ||
| encoding: Some("aesgcm".to_string()), | ||
| encryption: Some("salt=foo".to_string()), | ||
| encryption_key: None, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not fully isolated: