-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: read state #423
Merged
Merged
feat: read state #423
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
3d90779
fix: errors being excluded from metrics and not being logged
chris13524 3fd5553
feat: initial boilerplate
chris13524 59ac0a1
fix: tests and mark as read SQL
chris13524 5095d5a
fix: only same subscriber
chris13524 67388b8
fix: only mark unread notifications as read
chris13524 5de4386
chore: test mark as read
chris13524 3dd956f
feat: unread count in watchSubscriptions
chris13524 7658805
chore: more tests
chris13524 7794727
fix: count multiplying by scope count
chris13524 7b39e73
feat: has_more_unread mur
chris13524 6040e6f
chore: TODO
chris13524 fb7e55a
fix: index is_read
chris13524 b55800a
chore: remove comment
chris13524 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
ALTER TABLE subscriber_notification ADD COLUMN is_read BOOLEAN NOT NULL DEFAULT FALSE; | ||
chris13524 marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
use { | ||
crate::model::types::AccountId, | ||
parquet_derive::ParquetRecordWriter, | ||
relay_rpc::domain::{ProjectId, Topic}, | ||
serde::Serialize, | ||
std::sync::Arc, | ||
uuid::Uuid, | ||
}; | ||
|
||
pub struct MarkNotificationsAsReadParams { | ||
pub topic: Topic, | ||
pub message_id: Arc<str>, | ||
pub by_iss: Arc<str>, | ||
pub by_domain: Arc<str>, | ||
pub project_pk: Uuid, | ||
pub project_id: ProjectId, | ||
pub subscriber_pk: Uuid, | ||
pub subscriber_account: AccountId, | ||
pub notification_topic: Topic, | ||
pub subscriber_notification_pk: Uuid, | ||
pub notification_pk: Uuid, | ||
pub marked_count: usize, | ||
} | ||
|
||
#[derive(Debug, Serialize, ParquetRecordWriter)] | ||
pub struct MarkNotificationsAsRead { | ||
/// Time at which the event was generated | ||
pub event_at: chrono::NaiveDateTime, | ||
/// The relay topic used to manage the subscription that the get notifications request message was published to | ||
pub topic: Arc<str>, | ||
/// Relay message ID of request | ||
pub message_id: Arc<str>, | ||
/// JWT iss that made the request | ||
pub get_by_iss: Arc<str>, | ||
/// CACAO domain that made the request | ||
pub get_by_domain: Arc<str>, | ||
/// Primary key of the project in the Notify Server database that the subscriber is subscribed to | ||
pub project_pk: String, | ||
/// Project ID of the project that the subscriber is subscribed to | ||
pub project_id: Arc<str>, | ||
/// Primary Key of the subscriber in the Notify Server database | ||
pub subscriber_pk: String, | ||
/// Hash of the CAIP-10 account of the subscriber | ||
pub subscriber_account_hash: String, | ||
/// The topic that notifications are sent on | ||
pub notification_topic: Arc<str>, | ||
/// Primary key of the subscriber-specific notification in the Notify Server database | ||
pub subscriber_notification_pk: String, | ||
/// Primary key of the notification in the Notify Server database | ||
pub notification_pk: String, | ||
/// The total number of notifications returned in the request | ||
pub marked_count: usize, | ||
} | ||
|
||
impl From<MarkNotificationsAsReadParams> for MarkNotificationsAsRead { | ||
fn from(params: MarkNotificationsAsReadParams) -> Self { | ||
Self { | ||
event_at: wc::analytics::time::now(), | ||
topic: params.topic.into_value(), | ||
message_id: params.message_id, | ||
get_by_iss: params.by_iss, | ||
get_by_domain: params.by_domain, | ||
project_pk: params.project_pk.to_string(), | ||
project_id: params.project_id.into_value(), | ||
subscriber_pk: params.subscriber_pk.to_string(), | ||
subscriber_account_hash: sha256::digest(params.subscriber_account.as_ref()), | ||
notification_topic: params.notification_topic.into_value(), | ||
subscriber_notification_pk: params.subscriber_notification_pk.to_string(), | ||
notification_pk: params.notification_pk.to_string(), | ||
marked_count: params.marked_count, | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,10 @@ use { | |
error::NotifyServerError, | ||
metrics::Metrics, | ||
model::{ | ||
helpers::{GetNotificationsParams, GetNotificationsResult}, | ||
helpers::{ | ||
GetNotificationsParams, GetNotificationsResult, MarkNotificationsAsReadParams, | ||
MarkNotificationsAsReadParamsValidatorContext, | ||
}, | ||
types::{AccountId, AccountIdParseError}, | ||
}, | ||
registry::storage::{error::StorageError, redis::Redis, KeyValueStorage}, | ||
|
@@ -43,7 +46,7 @@ use { | |
tracing::{debug, info, warn}, | ||
url::Url, | ||
uuid::Uuid, | ||
validator::Validate, | ||
validator::{Validate, ValidateArgs}, | ||
x25519_dalek::{PublicKey, StaticSecret}, | ||
}; | ||
|
||
|
@@ -134,6 +137,8 @@ pub struct NotifyServerSubscription { | |
pub scope: HashSet<Uuid>, // TODO 15 hard limit | ||
/// Unix timestamp of expiration | ||
pub expiry: u64, | ||
/// Number of unread notifications | ||
pub unread_notification_count: u64, | ||
} | ||
|
||
impl GetSharedClaims for WatchSubscriptionsResponseAuth { | ||
|
@@ -313,7 +318,7 @@ pub struct SubscriptionGetNotificationsRequestAuth { | |
/// did:web of app domain | ||
pub app: DidWeb, | ||
#[serde(flatten)] | ||
#[validate] | ||
#[validate(nested)] | ||
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. Fix for silent breaking change in 0.17.0. See Keats/validator#307 |
||
pub params: GetNotificationsParams, | ||
} | ||
|
||
|
@@ -348,6 +353,52 @@ impl GetSharedClaims for SubscriptionGetNotificationsResponseAuth { | |
} | ||
} | ||
|
||
#[derive(Debug, Clone, Serialize, Deserialize)] | ||
pub struct SubscriptionMarkNotificationsAsReadRequestAuth { | ||
#[serde(flatten)] | ||
pub shared_claims: SharedClaims, | ||
/// ksu - key server for identity key verification | ||
pub ksu: String, | ||
/// did:pkh | ||
pub sub: String, | ||
/// did:web of app domain | ||
pub app: DidWeb, | ||
#[serde(flatten)] | ||
pub params: MarkNotificationsAsReadParams, | ||
} | ||
|
||
impl SubscriptionMarkNotificationsAsReadRequestAuth { | ||
pub fn validate(&self) -> Result<(), NotifyServerError> { | ||
self.params | ||
.validate_with_args(&MarkNotificationsAsReadParamsValidatorContext { | ||
all: self.params.all, | ||
}) | ||
.map_err(|error| NotifyServerError::UnprocessableEntity(error.to_string())) | ||
} | ||
} | ||
|
||
impl GetSharedClaims for SubscriptionMarkNotificationsAsReadRequestAuth { | ||
fn get_shared_claims(&self) -> &SharedClaims { | ||
&self.shared_claims | ||
} | ||
} | ||
|
||
#[derive(Debug, Clone, Serialize, Deserialize)] | ||
pub struct SubscriptionMarkNotificationsAsReadResponseAuth { | ||
#[serde(flatten)] | ||
pub shared_claims: SharedClaims, | ||
/// did:pkh | ||
pub sub: String, | ||
/// did:web of app domain | ||
pub app: DidWeb, | ||
} | ||
|
||
impl GetSharedClaims for SubscriptionMarkNotificationsAsReadResponseAuth { | ||
fn get_shared_claims(&self) -> &SharedClaims { | ||
&self.shared_claims | ||
} | ||
} | ||
|
||
#[derive(Debug, Error)] | ||
pub enum JwtError { | ||
#[error("Missing message part")] | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Upgrade to support custom validators accepting
Option<T>
instead ofT
, allowing the ability to validate the state of theOption
and not justT
.Some may consider this change a bug