Simplify notification serialization

Max Brunsfeld created

Change summary

crates/collab/src/db.rs                        |  3 
crates/collab/src/db/queries/notifications.rs  |  8 +-
crates/notifications/src/notification_store.rs |  8 --
crates/rpc/src/notification.rs                 | 50 ++++++++-----------
4 files changed, 29 insertions(+), 40 deletions(-)

Detailed changes

crates/collab/src/db.rs 🔗

@@ -13,6 +13,7 @@ use anyhow::anyhow;
 use collections::{BTreeMap, HashMap, HashSet};
 use dashmap::DashMap;
 use futures::StreamExt;
+use queries::channels::ChannelGraph;
 use rand::{prelude::StdRng, Rng, SeedableRng};
 use rpc::{
     proto::{self},
@@ -47,8 +48,6 @@ pub use ids::*;
 pub use sea_orm::ConnectOptions;
 pub use tables::user::Model as User;
 
-use self::queries::channels::ChannelGraph;
-
 pub struct Database {
     options: ConnectOptions,
     pool: DatabaseConnection,

crates/collab/src/db/queries/notifications.rs 🔗

@@ -76,10 +76,10 @@ impl Database {
         notification: Notification,
         tx: &DatabaseTransaction,
     ) -> Result<proto::Notification> {
-        let notification = notification.to_any();
+        let notification = notification.to_proto();
         let kind = *self
             .notification_kinds_by_name
-            .get(notification.kind.as_ref())
+            .get(&notification.kind)
             .ok_or_else(|| anyhow!("invalid notification kind {:?}", notification.kind))?;
 
         let model = notification::ActiveModel {
@@ -110,10 +110,10 @@ impl Database {
         notification: Notification,
         tx: &DatabaseTransaction,
     ) -> Result<Option<NotificationId>> {
-        let notification = notification.to_any();
+        let notification = notification.to_proto();
         let kind = *self
             .notification_kinds_by_name
-            .get(notification.kind.as_ref())
+            .get(&notification.kind)
             .ok_or_else(|| anyhow!("invalid notification kind {:?}", notification.kind))?;
         let actor_id = notification.actor_id.map(|id| UserId::from_proto(id));
         let notification = notification::Entity::find()

crates/notifications/src/notification_store.rs 🔗

@@ -4,7 +4,7 @@ use client::{Client, UserStore};
 use collections::HashMap;
 use db::smol::stream::StreamExt;
 use gpui::{AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, Task};
-use rpc::{proto, AnyNotification, Notification, TypedEnvelope};
+use rpc::{proto, Notification, TypedEnvelope};
 use std::{ops::Range, sync::Arc};
 use sum_tree::{Bias, SumTree};
 use time::OffsetDateTime;
@@ -185,11 +185,7 @@ impl NotificationStore {
                     is_read: message.is_read,
                     timestamp: OffsetDateTime::from_unix_timestamp(message.timestamp as i64)
                         .ok()?,
-                    notification: Notification::from_any(&AnyNotification {
-                        actor_id: message.actor_id,
-                        kind: message.kind.into(),
-                        content: message.content,
-                    })?,
+                    notification: Notification::from_proto(&message)?,
                 })
             })
             .collect::<Vec<_>>();

crates/rpc/src/notification.rs 🔗

@@ -1,7 +1,7 @@
+use crate::proto;
 use serde::{Deserialize, Serialize};
 use serde_json::{map, Value};
-use std::borrow::Cow;
-use strum::{EnumVariantNames, IntoStaticStr, VariantNames as _};
+use strum::{EnumVariantNames, VariantNames as _};
 
 const KIND: &'static str = "kind";
 const ACTOR_ID: &'static str = "actor_id";
@@ -9,10 +9,12 @@ const ACTOR_ID: &'static str = "actor_id";
 /// A notification that can be stored, associated with a given user.
 ///
 /// This struct is stored in the collab database as JSON, so it shouldn't be
-/// changed in a backward-incompatible way.
+/// changed in a backward-incompatible way. For example, when renaming a
+/// variant, add a serde alias for the old name.
 ///
-/// For example, when renaming a variant, add a serde alias for the old name.
-#[derive(Debug, Clone, PartialEq, Eq, EnumVariantNames, IntoStaticStr, Serialize, Deserialize)]
+/// When a notification is initiated by a user, use the `actor_id` field
+/// to store the user's id.
+#[derive(Debug, Clone, PartialEq, Eq, EnumVariantNames, Serialize, Deserialize)]
 #[serde(tag = "kind")]
 pub enum Notification {
     ContactRequest {
@@ -32,36 +34,28 @@ pub enum Notification {
     },
 }
 
-/// The representation of a notification that is stored in the database and
-/// sent over the wire.
-#[derive(Debug)]
-pub struct AnyNotification {
-    pub kind: Cow<'static, str>,
-    pub actor_id: Option<u64>,
-    pub content: String,
-}
-
 impl Notification {
-    pub fn to_any(&self) -> AnyNotification {
-        let kind: &'static str = self.into();
+    pub fn to_proto(&self) -> proto::Notification {
         let mut value = serde_json::to_value(self).unwrap();
         let mut actor_id = None;
-        if let Some(value) = value.as_object_mut() {
-            value.remove(KIND);
-            if let map::Entry::Occupied(e) = value.entry(ACTOR_ID) {
-                if e.get().is_u64() {
-                    actor_id = e.remove().as_u64();
-                }
+        let value = value.as_object_mut().unwrap();
+        let Some(Value::String(kind)) = value.remove(KIND) else {
+            unreachable!()
+        };
+        if let map::Entry::Occupied(e) = value.entry(ACTOR_ID) {
+            if e.get().is_u64() {
+                actor_id = e.remove().as_u64();
             }
         }
-        AnyNotification {
-            kind: Cow::Borrowed(kind),
+        proto::Notification {
+            kind,
             actor_id,
             content: serde_json::to_string(&value).unwrap(),
+            ..Default::default()
         }
     }
 
-    pub fn from_any(notification: &AnyNotification) -> Option<Self> {
+    pub fn from_proto(notification: &proto::Notification) -> Option<Self> {
         let mut value = serde_json::from_str::<Value>(&notification.content).ok()?;
         let object = value.as_object_mut()?;
         object.insert(KIND.into(), notification.kind.to_string().into());
@@ -92,13 +86,13 @@ fn test_notification() {
             message_id: 1,
         },
     ] {
-        let serialized = notification.to_any();
-        let deserialized = Notification::from_any(&serialized).unwrap();
+        let message = notification.to_proto();
+        let deserialized = Notification::from_proto(&message).unwrap();
         assert_eq!(deserialized, notification);
     }
 
     // When notifications are serialized, the `kind` and `actor_id` fields are
     // stored separately, and do not appear redundantly in the JSON.
     let notification = Notification::ContactRequest { actor_id: 1 };
-    assert_eq!(notification.to_any().content, "{}");
+    assert_eq!(notification.to_proto().content, "{}");
 }