separate account_data service methods for room vs global events

Previously we were mashing everything together as RoomAccountDataEvent,
even the global events. This technically worked, because of the hidden
custom fields on the ruma event types, but it's confusing and easy to
mess up. Separate methods with appropriate types are preferable.

src/api/client_server/account.rs

@@ -12,9 +12,12 @@ use ruma::{
         uiaa::{AuthFlow, AuthType, UiaaInfo},
     },
     events::{
-        room::message::RoomMessageEventContent, GlobalAccountDataEventType,
+        room::message::RoomMessageEventContent, AnyGlobalAccountDataEvent,
+        GlobalAccountDataEventType,
     },
-    push, UserId,
+    push,
+    serde::Raw,
+    UserId,
 };
 use tracing::{info, warn};
 
@@ -234,16 +237,16 @@ pub(crate) async fn register_route(
     services().users.set_displayname(&user_id, Some(displayname.clone()))?;
 
     // Initial account data
-    services().account_data.update(
-        None,
+    services().account_data.update_global(
         &user_id,
-        GlobalAccountDataEventType::PushRules.to_string().into(),
-        &serde_json::to_value(ruma::events::push_rules::PushRulesEvent {
+        &GlobalAccountDataEventType::PushRules,
+        &Raw::new(&ruma::events::push_rules::PushRulesEvent {
             content: ruma::events::push_rules::PushRulesEventContent {
                 global: push::Ruleset::server_default(&user_id),
             },
         })
-        .expect("to json always works"),
+        .expect("constructed event should be valid")
+        .cast::<AnyGlobalAccountDataEvent>(),
     )?;
 
     // Inhibit login does not work for guests

src/api/client_server/config.rs

@@ -7,12 +7,13 @@ use ruma::{
         error::ErrorKind,
     },
     events::{
-        AnyGlobalAccountDataEventContent, AnyRoomAccountDataEventContent,
+        AnyGlobalAccountDataEvent, AnyGlobalAccountDataEventContent,
+        AnyRoomAccountDataEvent, AnyRoomAccountDataEventContent,
     },
     serde::Raw,
 };
 use serde::Deserialize;
-use serde_json::{json, value::RawValue as RawJsonValue};
+use serde_json::json;
 
 use crate::{services, Ar, Error, Ra, Result};
 
@@ -24,21 +25,17 @@ pub(crate) async fn set_global_account_data_route(
 ) -> Result<Ra<set_global_account_data::v3::Response>> {
     let sender_user = body.sender_user.as_ref().expect("user is authenticated");
 
-    let data: serde_json::Value = serde_json::from_str(body.data.json().get())
-        .map_err(|_| {
-            Error::BadRequest(ErrorKind::BadJson, "Data is invalid.")
-        })?;
+    let event = Raw::new(&json!({
+        "type": &body.event_type,
+        "content": &body.data,
+    }))
+    .map_err(|_| Error::BadRequest(ErrorKind::BadJson, "Data is invalid."))?
+    .cast::<AnyGlobalAccountDataEvent>();
 
-    let event_type = body.event_type.to_string();
-
-    services().account_data.update(
-        None,
+    services().account_data.update_global(
         sender_user,
-        event_type.clone().into(),
-        &json!({
-            "type": event_type,
-            "content": data,
-        }),
+        &body.event_type,
+        &event,
     )?;
 
     Ok(Ra(set_global_account_data::v3::Response {}))
@@ -52,21 +49,18 @@ pub(crate) async fn set_room_account_data_route(
 ) -> Result<Ra<set_room_account_data::v3::Response>> {
     let sender_user = body.sender_user.as_ref().expect("user is authenticated");
 
-    let data: serde_json::Value = serde_json::from_str(body.data.json().get())
-        .map_err(|_| {
-            Error::BadRequest(ErrorKind::BadJson, "Data is invalid.")
-        })?;
+    let event = Raw::new(&json!({
+        "type": &body.event_type,
+        "content": &body.data,
+    }))
+    .map_err(|_| Error::BadRequest(ErrorKind::BadJson, "Data is invalid."))?
+    .cast::<AnyRoomAccountDataEvent>();
 
-    let event_type = body.event_type.to_string();
-
-    services().account_data.update(
-        Some(&body.room_id),
+    services().account_data.update_room(
+        &body.room_id,
         sender_user,
-        event_type.clone().into(),
-        &json!({
-            "type": event_type,
-            "content": data,
-        }),
+        &body.event_type,
+        &event,
     )?;
 
     Ok(Ra(set_room_account_data::v3::Response {}))
@@ -80,17 +74,15 @@ pub(crate) async fn get_global_account_data_route(
 ) -> Result<Ra<get_global_account_data::v3::Response>> {
     let sender_user = body.sender_user.as_ref().expect("user is authenticated");
 
-    let event: Box<RawJsonValue> = services()
+    let event = services()
         .account_data
-        .get(None, sender_user, body.event_type.to_string().into())?
+        .get_global(sender_user, &body.event_type)?
         .ok_or(Error::BadRequest(ErrorKind::NotFound, "Data not found."))?;
 
-    let account_data =
-        serde_json::from_str::<ExtractGlobalEventContent>(event.get())
-            .map_err(|_| {
-                Error::bad_database("Invalid account data event in db.")
-            })?
-            .content;
+    let account_data = event
+        .deserialize_as::<ExtractGlobalEventContent>()
+        .map_err(|_| Error::bad_database("Invalid account data event in db."))?
+        .content;
 
     Ok(Ra(get_global_account_data::v3::Response {
         account_data,
@@ -105,17 +97,15 @@ pub(crate) async fn get_room_account_data_route(
 ) -> Result<Ra<get_room_account_data::v3::Response>> {
     let sender_user = body.sender_user.as_ref().expect("user is authenticated");
 
-    let event: Box<RawJsonValue> = services()
+    let event = services()
         .account_data
-        .get(Some(&body.room_id), sender_user, body.event_type.clone())?
+        .get_room(&body.room_id, sender_user, &body.event_type)?
         .ok_or(Error::BadRequest(ErrorKind::NotFound, "Data not found."))?;
 
-    let account_data =
-        serde_json::from_str::<ExtractRoomEventContent>(event.get())
-            .map_err(|_| {
-                Error::bad_database("Invalid account data event in db.")
-            })?
-            .content;
+    let account_data = event
+        .deserialize_as::<ExtractRoomEventContent>()
+        .map_err(|_| Error::bad_database("Invalid account data event in db."))?
+        .content;
 
     Ok(Ra(get_room_account_data::v3::Response {
         account_data,

src/api/client_server/push.rs

@@ -7,8 +7,12 @@ use ruma::{
             set_pushrule_actions, set_pushrule_enabled,
         },
     },
-    events::{push_rules::PushRulesEvent, GlobalAccountDataEventType},
+    events::{
+        push_rules::PushRulesEvent, AnyGlobalAccountDataEvent,
+        GlobalAccountDataEventType,
+    },
     push::{AnyPushRuleRef, InsertPushRuleError, RemovePushRuleError},
+    serde::Raw,
 };
 
 use crate::{services, Ar, Error, Ra, Result};
@@ -23,17 +27,14 @@ pub(crate) async fn get_pushrules_all_route(
 
     let event = services()
         .account_data
-        .get(
-            None,
-            sender_user,
-            GlobalAccountDataEventType::PushRules.to_string().into(),
-        )?
+        .get_global(sender_user, &GlobalAccountDataEventType::PushRules)?
         .ok_or(Error::BadRequest(
             ErrorKind::NotFound,
             "PushRules event not found.",
         ))?;
 
-    let account_data = serde_json::from_str::<PushRulesEvent>(event.get())
+    let account_data = event
+        .deserialize_as::<PushRulesEvent>()
         .map_err(|_| Error::bad_database("Invalid account data event in db."))?
         .content;
 
@@ -52,17 +53,14 @@ pub(crate) async fn get_pushrule_route(
 
     let event = services()
         .account_data
-        .get(
-            None,
-            sender_user,
-            GlobalAccountDataEventType::PushRules.to_string().into(),
-        )?
+        .get_global(sender_user, &GlobalAccountDataEventType::PushRules)?
         .ok_or(Error::BadRequest(
             ErrorKind::NotFound,
             "PushRules event not found.",
         ))?;
 
-    let account_data = serde_json::from_str::<PushRulesEvent>(event.get())
+    let account_data = event
+        .deserialize_as::<PushRulesEvent>()
         .map_err(|_| Error::bad_database("Invalid account data event in db."))?
         .content;
 
@@ -91,18 +89,14 @@ pub(crate) async fn set_pushrule_route(
 
     let event = services()
         .account_data
-        .get(
-            None,
-            sender_user,
-            GlobalAccountDataEventType::PushRules.to_string().into(),
-        )?
+        .get_global(sender_user, &GlobalAccountDataEventType::PushRules)?
         .ok_or(Error::BadRequest(
             ErrorKind::NotFound,
             "PushRules event not found.",
         ))?;
 
-    let mut account_data = serde_json::from_str::<PushRulesEvent>(event.get())
-        .map_err(|_| {
+    let mut account_data =
+        event.deserialize_as::<PushRulesEvent>().map_err(|_| {
             Error::bad_database("Invalid account data event in db.")
         })?;
 
@@ -142,12 +136,12 @@ pub(crate) async fn set_pushrule_route(
         return Err(err);
     }
 
-    services().account_data.update(
-        None,
+    services().account_data.update_global(
         sender_user,
-        GlobalAccountDataEventType::PushRules.to_string().into(),
-        &serde_json::to_value(account_data)
-            .expect("to json value always works"),
+        &GlobalAccountDataEventType::PushRules,
+        &Raw::new(&account_data)
+            .expect("json event serialization should always succeed")
+            .cast::<AnyGlobalAccountDataEvent>(),
     )?;
 
     Ok(Ra(set_pushrule::v3::Response {}))
@@ -163,17 +157,14 @@ pub(crate) async fn get_pushrule_actions_route(
 
     let event = services()
         .account_data
-        .get(
-            None,
-            sender_user,
-            GlobalAccountDataEventType::PushRules.to_string().into(),
-        )?
+        .get_global(sender_user, &GlobalAccountDataEventType::PushRules)?
         .ok_or(Error::BadRequest(
             ErrorKind::NotFound,
             "PushRules event not found.",
         ))?;
 
-    let account_data = serde_json::from_str::<PushRulesEvent>(event.get())
+    let account_data = event
+        .deserialize_as::<PushRulesEvent>()
         .map_err(|_| Error::bad_database("Invalid account data event in db."))?
         .content;
 
@@ -201,18 +192,14 @@ pub(crate) async fn set_pushrule_actions_route(
 
     let event = services()
         .account_data
-        .get(
-            None,
-            sender_user,
-            GlobalAccountDataEventType::PushRules.to_string().into(),
-        )?
+        .get_global(sender_user, &GlobalAccountDataEventType::PushRules)?
         .ok_or(Error::BadRequest(
             ErrorKind::NotFound,
             "PushRules event not found.",
         ))?;
 
-    let mut account_data = serde_json::from_str::<PushRulesEvent>(event.get())
-        .map_err(|_| {
+    let mut account_data =
+        event.deserialize_as::<PushRulesEvent>().map_err(|_| {
             Error::bad_database("Invalid account data event in db.")
         })?;
 
@@ -228,12 +215,12 @@ pub(crate) async fn set_pushrule_actions_route(
         ));
     }
 
-    services().account_data.update(
-        None,
+    services().account_data.update_global(
         sender_user,
-        GlobalAccountDataEventType::PushRules.to_string().into(),
-        &serde_json::to_value(account_data)
-            .expect("to json value always works"),
+        &GlobalAccountDataEventType::PushRules,
+        &Raw::new(&account_data)
+            .expect("json event serialization should always suceed")
+            .cast::<AnyGlobalAccountDataEvent>(),
     )?;
 
     Ok(Ra(set_pushrule_actions::v3::Response {}))
@@ -249,18 +236,14 @@ pub(crate) async fn get_pushrule_enabled_route(
 
     let event = services()
         .account_data
-        .get(
-            None,
-            sender_user,
-            GlobalAccountDataEventType::PushRules.to_string().into(),
-        )?
+        .get_global(sender_user, &GlobalAccountDataEventType::PushRules)?
         .ok_or(Error::BadRequest(
             ErrorKind::NotFound,
             "PushRules event not found.",
         ))?;
 
-    let account_data = serde_json::from_str::<PushRulesEvent>(event.get())
-        .map_err(|_| {
+    let account_data =
+        event.deserialize_as::<PushRulesEvent>().map_err(|_| {
             Error::bad_database("Invalid account data event in db.")
         })?;
 
@@ -288,18 +271,14 @@ pub(crate) async fn set_pushrule_enabled_route(
 
     let event = services()
         .account_data
-        .get(
-            None,
-            sender_user,
-            GlobalAccountDataEventType::PushRules.to_string().into(),
-        )?
+        .get_global(sender_user, &GlobalAccountDataEventType::PushRules)?
         .ok_or(Error::BadRequest(
             ErrorKind::NotFound,
             "PushRules event not found.",
         ))?;
 
-    let mut account_data = serde_json::from_str::<PushRulesEvent>(event.get())
-        .map_err(|_| {
+    let mut account_data =
+        event.deserialize_as::<PushRulesEvent>().map_err(|_| {
             Error::bad_database("Invalid account data event in db.")
         })?;
 
@@ -315,12 +294,12 @@ pub(crate) async fn set_pushrule_enabled_route(
         ));
     }
 
-    services().account_data.update(
-        None,
+    services().account_data.update_global(
         sender_user,
-        GlobalAccountDataEventType::PushRules.to_string().into(),
-        &serde_json::to_value(account_data)
-            .expect("to json value always works"),
+        &GlobalAccountDataEventType::PushRules,
+        &Raw::new(&account_data)
+            .expect("json event serialization should always succeed")
+            .cast::<AnyGlobalAccountDataEvent>(),
     )?;
 
     Ok(Ra(set_pushrule_enabled::v3::Response {}))
@@ -336,18 +315,14 @@ pub(crate) async fn delete_pushrule_route(
 
     let event = services()
         .account_data
-        .get(
-            None,
-            sender_user,
-            GlobalAccountDataEventType::PushRules.to_string().into(),
-        )?
+        .get_global(sender_user, &GlobalAccountDataEventType::PushRules)?
         .ok_or(Error::BadRequest(
             ErrorKind::NotFound,
             "PushRules event not found.",
         ))?;
 
-    let mut account_data = serde_json::from_str::<PushRulesEvent>(event.get())
-        .map_err(|_| {
+    let mut account_data =
+        event.deserialize_as::<PushRulesEvent>().map_err(|_| {
             Error::bad_database("Invalid account data event in db.")
         })?;
 
@@ -368,12 +343,12 @@ pub(crate) async fn delete_pushrule_route(
         return Err(err);
     }
 
-    services().account_data.update(
-        None,
+    services().account_data.update_global(
         sender_user,
-        GlobalAccountDataEventType::PushRules.to_string().into(),
-        &serde_json::to_value(account_data)
-            .expect("to json value always works"),
+        &GlobalAccountDataEventType::PushRules,
+        &Raw::new(&account_data)
+            .expect("json event serialization should always suceed")
+            .cast::<AnyGlobalAccountDataEvent>(),
     )?;
 
     Ok(Ra(delete_pushrule::v3::Response {}))

src/api/client_server/read_marker.rs

@@ -6,8 +6,9 @@ use ruma::{
     },
     events::{
         receipt::{ReceiptThread, ReceiptType},
-        RoomAccountDataEventType,
+        AnyRoomAccountDataEvent, RoomAccountDataEventType,
     },
+    serde::Raw,
     MilliSecondsSinceUnixEpoch,
 };
 
@@ -33,12 +34,13 @@ pub(crate) async fn set_read_marker_route(
                 event_id: fully_read.clone(),
             },
         };
-        services().account_data.update(
-            Some(&body.room_id),
+        services().account_data.update_room(
+            &body.room_id,
             sender_user,
-            RoomAccountDataEventType::FullyRead,
-            &serde_json::to_value(fully_read_event)
-                .expect("to json value always works"),
+            &RoomAccountDataEventType::FullyRead,
+            &Raw::new(&fully_read_event)
+                .expect("json event serialization should always suceed")
+                .cast::<AnyRoomAccountDataEvent>(),
         )?;
     }
 
@@ -129,12 +131,13 @@ pub(crate) async fn create_receipt_route(
                     event_id: body.event_id.clone(),
                 },
             };
-            services().account_data.update(
-                Some(&body.room_id),
+            services().account_data.update_room(
+                &body.room_id,
                 sender_user,
-                RoomAccountDataEventType::FullyRead,
-                &serde_json::to_value(fully_read_event)
-                    .expect("to json value always works"),
+                &RoomAccountDataEventType::FullyRead,
+                &Raw::new(&fully_read_event)
+                    .expect("json event serialization should always succeed")
+                    .cast::<AnyRoomAccountDataEvent>(),
             )?;
         }
         create_receipt::v3::ReceiptType::Read => {

src/api/client_server/sync/msc3575.rs

@@ -644,17 +644,8 @@ pub(crate) async fn sync_events_v4_route(
                 {
                     services()
                         .account_data
-                        .changes_since(None, &sender_user, globalsince)?
-                        .into_iter()
-                        .filter_map(|(_, v)| {
-                            serde_json::from_str(v.json().get())
-                                .map_err(|_| {
-                                    Error::bad_database(
-                                        "Invalid account event in database.",
-                                    )
-                                })
-                                .ok()
-                        })
+                        .global_changes_since(&sender_user, globalsince)?
+                        .into_values()
                         .collect()
                 } else {
                     Vec::new()

src/api/client_server/sync/v3.rs

@@ -234,17 +234,8 @@ pub(crate) async fn sync_events_route(
         account_data: GlobalAccountData {
             events: services()
                 .account_data
-                .changes_since(None, ctx.sender_user, ctx.since)?
-                .into_iter()
-                .filter_map(|(_, v)| {
-                    serde_json::from_str(v.json().get())
-                        .map_err(|_| {
-                            Error::bad_database(
-                                "Invalid account event in database.",
-                            )
-                        })
-                        .ok()
-                })
+                .global_changes_since(ctx.sender_user, ctx.since)?
+                .into_values()
                 .collect(),
         },
         device_lists: DeviceLists {
@@ -880,17 +871,8 @@ async fn load_joined_room(
         account_data: RoomAccountData {
             events: services()
                 .account_data
-                .changes_since(Some(room_id), ctx.sender_user, ctx.since)?
-                .into_iter()
-                .filter_map(|(_, v)| {
-                    serde_json::from_str(v.json().get())
-                        .map_err(|_| {
-                            Error::bad_database(
-                                "Invalid account event in database.",
-                            )
-                        })
-                        .ok()
-                })
+                .room_changes_since(ctx.sender_user, room_id, ctx.since)?
+                .into_values()
                 .collect(),
         },
         summary: RoomSummary {

src/api/client_server/tag.rs

@@ -4,8 +4,9 @@ use ruma::{
     api::client::tag::{create_tag, delete_tag, get_tags},
     events::{
         tag::{TagEvent, TagEventContent},
-        RoomAccountDataEventType,
+        AnyRoomAccountDataEvent, RoomAccountDataEventType,
     },
+    serde::Raw,
 };
 
 use crate::{services, Ar, Error, Ra, Result};
@@ -20,10 +21,10 @@ pub(crate) async fn update_tag_route(
 ) -> Result<Ra<create_tag::v3::Response>> {
     let sender_user = body.sender_user.as_ref().expect("user is authenticated");
 
-    let event = services().account_data.get(
-        Some(&body.room_id),
+    let event = services().account_data.get_room(
+        &body.room_id,
         sender_user,
-        RoomAccountDataEventType::Tag,
+        &RoomAccountDataEventType::Tag,
     )?;
 
     let mut tags_event = event.map_or_else(
@@ -35,7 +36,7 @@ pub(crate) async fn update_tag_route(
             })
         },
         |e| {
-            serde_json::from_str(e.get()).map_err(|_| {
+            e.deserialize_as::<TagEvent>().map_err(|_| {
                 Error::bad_database("Invalid account data event in db.")
             })
         },
@@ -46,11 +47,13 @@ pub(crate) async fn update_tag_route(
         .tags
         .insert(body.tag.clone().into(), body.tag_info.clone());
 
-    services().account_data.update(
-        Some(&body.room_id),
+    services().account_data.update_room(
+        &body.room_id,
         sender_user,
-        RoomAccountDataEventType::Tag,
-        &serde_json::to_value(tags_event).expect("to json value always works"),
+        &RoomAccountDataEventType::Tag,
+        &Raw::new(&tags_event)
+            .expect("json event serialization should always suceed")
+            .cast::<AnyRoomAccountDataEvent>(),
     )?;
 
     Ok(Ra(create_tag::v3::Response {}))
@@ -66,10 +69,10 @@ pub(crate) async fn delete_tag_route(
 ) -> Result<Ra<delete_tag::v3::Response>> {
     let sender_user = body.sender_user.as_ref().expect("user is authenticated");
 
-    let event = services().account_data.get(
-        Some(&body.room_id),
+    let event = services().account_data.get_room(
+        &body.room_id,
         sender_user,
-        RoomAccountDataEventType::Tag,
+        &RoomAccountDataEventType::Tag,
     )?;
 
     let mut tags_event = event.map_or_else(
@@ -81,7 +84,7 @@ pub(crate) async fn delete_tag_route(
             })
         },
         |e| {
-            serde_json::from_str(e.get()).map_err(|_| {
+            e.deserialize_as::<TagEvent>().map_err(|_| {
                 Error::bad_database("Invalid account data event in db.")
             })
         },
@@ -89,11 +92,13 @@ pub(crate) async fn delete_tag_route(
 
     tags_event.content.tags.remove(&body.tag.clone().into());
 
-    services().account_data.update(
-        Some(&body.room_id),
+    services().account_data.update_room(
+        &body.room_id,
         sender_user,
-        RoomAccountDataEventType::Tag,
-        &serde_json::to_value(tags_event).expect("to json value always works"),
+        &RoomAccountDataEventType::Tag,
+        &Raw::new(&tags_event)
+            .expect("json value serialization should always succeed")
+            .cast::<AnyRoomAccountDataEvent>(),
     )?;
 
     Ok(Ra(delete_tag::v3::Response {}))
@@ -109,10 +114,10 @@ pub(crate) async fn get_tags_route(
 ) -> Result<Ra<get_tags::v3::Response>> {
     let sender_user = body.sender_user.as_ref().expect("user is authenticated");
 
-    let event = services().account_data.get(
-        Some(&body.room_id),
+    let event = services().account_data.get_room(
+        &body.room_id,
         sender_user,
-        RoomAccountDataEventType::Tag,
+        &RoomAccountDataEventType::Tag,
     )?;
 
     let tags_event = event.map_or_else(
@@ -124,7 +129,7 @@ pub(crate) async fn get_tags_route(
             })
         },
         |e| {
-            serde_json::from_str(e.get()).map_err(|_| {
+            e.deserialize_as::<TagEvent>().map_err(|_| {
                 Error::bad_database("Invalid account data event in db.")
             })
         },