From 94990f60f2b1c4337e08de9be43c8079e97a58bb Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Tue, 8 Nov 2022 19:32:53 +0100 Subject: [PATCH] events: Don't skip serializing empty content of redacted events --- crates/ruma-common/src/events/_custom.rs | 22 +---- crates/ruma-common/src/events/content.rs | 36 +------ crates/ruma-common/src/events/room/aliases.rs | 15 +-- crates/ruma-common/src/events/room/member.rs | 18 +--- crates/ruma-common/tests/events/redacted.rs | 94 ++++++++++--------- crates/ruma-macros/src/events/event.rs | 29 ++---- .../ruma-macros/src/events/event_content.rs | 54 +---------- 7 files changed, 77 insertions(+), 191 deletions(-) diff --git a/crates/ruma-common/src/events/_custom.rs b/crates/ruma-common/src/events/_custom.rs index 7d870fff..b8cd18ab 100644 --- a/crates/ruma-common/src/events/_custom.rs +++ b/crates/ruma-common/src/events/_custom.rs @@ -3,10 +3,10 @@ use serde_json::value::RawValue as RawJsonValue; use super::{ EphemeralRoomEventContent, EphemeralRoomEventType, EventContent, GlobalAccountDataEventContent, - GlobalAccountDataEventType, HasDeserializeFields, MessageLikeEventContent, - MessageLikeEventType, RedactContent, RedactedEventContent, RedactedMessageLikeEventContent, - RedactedStateEventContent, RoomAccountDataEventContent, RoomAccountDataEventType, - StateEventContent, StateEventType, StateUnsigned, ToDeviceEventContent, ToDeviceEventType, + GlobalAccountDataEventType, MessageLikeEventContent, MessageLikeEventType, RedactContent, + RedactedEventContent, RedactedMessageLikeEventContent, RedactedStateEventContent, + RoomAccountDataEventContent, RoomAccountDataEventType, StateEventContent, StateEventType, + StateUnsigned, ToDeviceEventContent, ToDeviceEventType, }; use crate::RoomVersionId; @@ -48,19 +48,7 @@ macro_rules! custom_room_event_content { } } - impl RedactedEventContent for $i { - fn empty(event_type: &str) -> serde_json::Result { - Ok(Self { event_type: event_type.into() }) - } - - fn has_serialize_fields(&self) -> bool { - false - } - - fn has_deserialize_fields() -> HasDeserializeFields { - HasDeserializeFields::False - } - } + impl RedactedEventContent for $i {} }; } diff --git a/crates/ruma-common/src/events/content.rs b/crates/ruma-common/src/events/content.rs index 79c5ef26..5e1bf492 100644 --- a/crates/ruma-common/src/events/content.rs +++ b/crates/ruma-common/src/events/content.rs @@ -45,41 +45,7 @@ where /// `AnyMessageLikeEvent` and their "sync" and "stripped" counterparts. /// The `RedactedEventContent` trait is an implementation detail, ruma makes no /// API guarantees. -pub trait RedactedEventContent: EventContent { - /// Constructs the redacted event content. - /// - /// If called for anything but "empty" redacted content this will error. - #[doc(hidden)] - fn empty(_event_type: &str) -> serde_json::Result { - Err(serde::de::Error::custom("this event is not redacted")) - } - - /// Determines if the redacted event content needs to serialize fields. - #[doc(hidden)] - fn has_serialize_fields(&self) -> bool; - - /// Determines if the redacted event content needs to deserialize fields. - #[doc(hidden)] - fn has_deserialize_fields() -> HasDeserializeFields; -} - -/// `HasDeserializeFields` is used in the code generated by the `Event` derive -/// to aid in deserializing redacted events. -#[doc(hidden)] -#[derive(Debug)] -#[allow(clippy::exhaustive_enums)] -pub enum HasDeserializeFields { - /// Deserialize the event's content, failing if invalid. - True, - - /// Return the redacted version of this event's content. - False, - - /// `Optional` is used for `RedactedAliasesEventContent` since it has - /// an empty version and one with content left after redaction that - /// must be supported together. - Optional, -} +pub trait RedactedEventContent: EventContent {} /// Trait for abstracting over event content structs. /// diff --git a/crates/ruma-common/src/events/room/aliases.rs b/crates/ruma-common/src/events/room/aliases.rs index a5d2891c..667012e2 100644 --- a/crates/ruma-common/src/events/room/aliases.rs +++ b/crates/ruma-common/src/events/room/aliases.rs @@ -6,8 +6,8 @@ use serde_json::value::RawValue as RawJsonValue; use crate::{ events::{ - EventContent, HasDeserializeFields, RedactContent, RedactedEventContent, - RedactedStateEventContent, StateEventContent, StateEventType, StateUnsigned, + EventContent, RedactContent, RedactedEventContent, RedactedStateEventContent, + StateEventContent, StateEventType, StateUnsigned, }, OwnedRoomAliasId, OwnedServerName, RoomVersionId, }; @@ -57,6 +57,7 @@ pub struct RedactedRoomAliasesEventContent { /// /// According to the Matrix spec version 1 redaction rules allowed this field to be /// kept after redaction, this was changed in version 6. + #[serde(skip_serializing_if = "Option::is_none")] pub aliases: Option>, } @@ -105,12 +106,4 @@ impl RedactedStateEventContent for RedactedRoomAliasesEventContent {} // Since this redacted event has fields we leave the default `empty` method // that will error if called. -impl RedactedEventContent for RedactedRoomAliasesEventContent { - fn has_serialize_fields(&self) -> bool { - self.aliases.is_some() - } - - fn has_deserialize_fields() -> HasDeserializeFields { - HasDeserializeFields::Optional - } -} +impl RedactedEventContent for RedactedRoomAliasesEventContent {} diff --git a/crates/ruma-common/src/events/room/member.rs b/crates/ruma-common/src/events/room/member.rs index 9a664b2a..db03a8b6 100644 --- a/crates/ruma-common/src/events/room/member.rs +++ b/crates/ruma-common/src/events/room/member.rs @@ -11,9 +11,9 @@ use serde_json::{from_str as from_json_str, value::RawValue as RawJsonValue}; use crate::{ events::{ - AnyStrippedStateEvent, EventContent, HasDeserializeFields, RedactContent, - RedactedEventContent, RedactedStateEventContent, Relations, StateEventContent, - StateEventType, StateUnsigned, StateUnsignedFromParts, StaticEventContent, + AnyStrippedStateEvent, EventContent, RedactContent, RedactedEventContent, + RedactedStateEventContent, Relations, StateEventContent, StateEventType, StateUnsigned, + StateUnsignedFromParts, StaticEventContent, }, serde::{CanBeEmpty, Raw, StringEnum}, OwnedMxcUri, OwnedServerName, OwnedServerSigningKeyId, OwnedTransactionId, OwnedUserId, @@ -170,7 +170,7 @@ pub struct RedactedRoomMemberEventContent { /// /// This is redacted in room versions 8 and below. It is used for validating /// joins when the join rule is restricted. - #[serde(rename = "join_authorised_via_users_server")] + #[serde(rename = "join_authorised_via_users_server", skip_serializing_if = "Option::is_none")] pub join_authorized_via_users_server: Option, } @@ -218,15 +218,7 @@ impl RedactedStateEventContent for RedactedRoomMemberEventContent {} // Since this redacted event has fields we leave the default `empty` method // that will error if called. -impl RedactedEventContent for RedactedRoomMemberEventContent { - fn has_serialize_fields(&self) -> bool { - true - } - - fn has_deserialize_fields() -> HasDeserializeFields { - HasDeserializeFields::Optional - } -} +impl RedactedEventContent for RedactedRoomMemberEventContent {} impl RoomMemberEvent { /// Obtain the membership state, regardless of whether this event is redacted. diff --git a/crates/ruma-common/tests/events/redacted.rs b/crates/ruma-common/tests/events/redacted.rs index e0cae073..26751c30 100644 --- a/crates/ruma-common/tests/events/redacted.rs +++ b/crates/ruma-common/tests/events/redacted.rs @@ -49,10 +49,11 @@ fn redacted_message_event_serialize() { }; let expected = json!({ - "event_id": "$h29iv0s8:example.com", - "origin_server_ts": 1, - "sender": "@carl:example.com", - "type": "m.room.message", + "content": {}, + "event_id": "$h29iv0s8:example.com", + "origin_server_ts": 1, + "sender": "@carl:example.com", + "type": "m.room.message", }); let actual = to_json_value(&redacted).unwrap(); @@ -60,7 +61,7 @@ fn redacted_message_event_serialize() { } #[test] -fn redacted_aliases_event_serialize_no_content() { +fn redacted_aliases_event_serialize_empty_content() { let redacted = RedactedSyncStateEvent { content: RedactedRoomAliasesEventContent::default(), event_id: event_id!("$h29iv0s8:example.com").to_owned(), @@ -71,11 +72,12 @@ fn redacted_aliases_event_serialize_no_content() { }; let expected = json!({ - "event_id": "$h29iv0s8:example.com", - "state_key": "example.com", - "origin_server_ts": 1, - "sender": "@carl:example.com", - "type": "m.room.aliases", + "content": {}, + "event_id": "$h29iv0s8:example.com", + "state_key": "example.com", + "origin_server_ts": 1, + "sender": "@carl:example.com", + "type": "m.room.aliases", }); let actual = to_json_value(&redacted).unwrap(); @@ -94,14 +96,14 @@ fn redacted_aliases_event_serialize_with_content() { }; let expected = json!({ - "content": { - "aliases": [] - }, - "event_id": "$h29iv0s8:example.com", - "state_key": "example.com", - "origin_server_ts": 1, - "sender": "@carl:example.com", - "type": "m.room.aliases", + "content": { + "aliases": [] + }, + "event_id": "$h29iv0s8:example.com", + "state_key": "example.com", + "origin_server_ts": 1, + "sender": "@carl:example.com", + "type": "m.room.aliases", }); let actual = to_json_value(&redacted).unwrap(); @@ -111,12 +113,13 @@ fn redacted_aliases_event_serialize_with_content() { #[test] fn redacted_aliases_deserialize() { let redacted = json!({ - "event_id": "$h29iv0s8:example.com", - "origin_server_ts": 1, - "sender": "@carl:example.com", - "state_key": "hello", - "unsigned": unsigned(), - "type": "m.room.aliases", + "content": {}, + "event_id": "$h29iv0s8:example.com", + "origin_server_ts": 1, + "sender": "@carl:example.com", + "state_key": "hello", + "unsigned": unsigned(), + "type": "m.room.aliases", }); let actual = to_json_value(&redacted).unwrap(); @@ -134,12 +137,13 @@ fn redacted_aliases_deserialize() { #[test] fn redacted_deserialize_any_room() { let redacted = json!({ - "event_id": "$h29iv0s8:example.com", - "room_id": "!roomid:room.com", - "origin_server_ts": 1, - "sender": "@carl:example.com", - "unsigned": unsigned(), - "type": "m.room.message", + "content": {}, + "event_id": "$h29iv0s8:example.com", + "room_id": "!roomid:room.com", + "origin_server_ts": 1, + "sender": "@carl:example.com", + "unsigned": unsigned(), + "type": "m.room.message", }); let actual = to_json_value(&redacted).unwrap(); @@ -171,11 +175,12 @@ fn redacted_deserialize_any_room_sync() { }))); let redacted = json!({ - "event_id": "$h29iv0s8:example.com", - "origin_server_ts": 1, - "sender": "@carl:example.com", - "unsigned": unsigned, - "type": "m.room.message", + "content": {}, + "event_id": "$h29iv0s8:example.com", + "origin_server_ts": 1, + "sender": "@carl:example.com", + "unsigned": unsigned, + "type": "m.room.message", }); let actual = to_json_value(&redacted).unwrap(); @@ -192,15 +197,15 @@ fn redacted_deserialize_any_room_sync() { #[test] fn redacted_state_event_deserialize() { let redacted = json!({ - "content": { - "creator": "@carl:example.com", - }, - "event_id": "$h29iv0s8:example.com", - "origin_server_ts": 1, - "sender": "@carl:example.com", - "state_key": "", - "unsigned": unsigned(), - "type": "m.room.create", + "content": { + "creator": "@carl:example.com", + }, + "event_id": "$h29iv0s8:example.com", + "origin_server_ts": 1, + "sender": "@carl:example.com", + "state_key": "", + "unsigned": unsigned(), + "type": "m.room.create", }); let redacted = assert_matches!( @@ -217,6 +222,7 @@ fn redacted_state_event_deserialize() { #[test] fn redacted_custom_event_deserialize() { let redacted = json!({ + "content": {}, "event_id": "$h29iv0s8:example.com", "origin_server_ts": 1, "sender": "@carl:example.com", diff --git a/crates/ruma-macros/src/events/event.rs b/crates/ruma-macros/src/events/event.rs index 98693296..98e89d7c 100644 --- a/crates/ruma-macros/src/events/event.rs +++ b/crates/ruma-macros/src/events/event.rs @@ -80,9 +80,7 @@ fn expand_serialize_event( let name = field.ident.as_ref().unwrap(); if name == "content" && var.is_redacted() { quote! { - if #ruma_common::events::RedactedEventContent::has_serialize_fields(&self.content) { - state.serialize_field("content", &self.content)?; - } + state.serialize_field("content", &self.content)?; } } else if name == "unsigned" { quote! { @@ -188,25 +186,12 @@ fn expand_deserialize_event( Ok(if name == "content" { if is_generic && var.is_redacted() { quote! { - let content = match C::has_deserialize_fields() { - #ruma_common::events::HasDeserializeFields::False => { - C::empty(&event_type).map_err(#serde::de::Error::custom)? - }, - #ruma_common::events::HasDeserializeFields::True => { - let json = content.ok_or_else( - || #serde::de::Error::missing_field("content"), - )?; - C::from_parts(&event_type, &json) - .map_err(#serde::de::Error::custom)? - }, - #ruma_common::events::HasDeserializeFields::Optional => { - let json = content.unwrap_or( - #serde_json::value::RawValue::from_string("{}".to_owned()) - .unwrap() - ); - C::from_parts(&event_type, &json) - .map_err(#serde::de::Error::custom)? - }, + let content = { + let json = content.ok_or_else( + || #serde::de::Error::missing_field("content"), + )?; + C::from_parts(&event_type, &json) + .map_err(#serde::de::Error::custom)? }; } } else if is_generic { diff --git a/crates/ruma-macros/src/events/event_content.rs b/crates/ruma-macros/src/events/event_content.rs index 8aa321d5..dbf67c95 100644 --- a/crates/ruma-macros/src/events/event_content.rs +++ b/crates/ruma-macros/src/events/event_content.rs @@ -391,7 +391,6 @@ fn generate_redacted_event_content<'a>( ); let serde = quote! { #ruma_common::exports::serde }; - let serde_json = quote! { #ruma_common::exports::serde_json }; let doc = format!("Redacted form of [`{ident}`]"); let redacted_ident = format_ident!("Redacted{ident}"); @@ -428,34 +427,13 @@ fn generate_redacted_event_content<'a>( let redaction_struct_fields = kept_redacted_fields.iter().flat_map(|f| &f.ident); - let (redacted_fields, redacted_return) = if kept_redacted_fields.is_empty() { - (quote! { ; }, quote! { Ok(#redacted_ident {}) }) - } else { - ( - quote! { - { #( #kept_redacted_fields, )* } - }, - quote! { - Err(#serde::de::Error::custom( - format!("this redacted event has fields that cannot be constructed") - )) - }, - ) - }; - - let (has_deserialize_fields, has_serialize_fields) = if kept_redacted_fields.is_empty() { - (quote! { #ruma_common::events::HasDeserializeFields::False }, quote! { false }) - } else { - (quote! { #ruma_common::events::HasDeserializeFields::True }, quote! { true }) - }; - let constructor = kept_redacted_fields.is_empty().then(|| { let doc = format!("Creates an empty {redacted_ident}."); quote! { impl #redacted_ident { #[doc = #doc] pub fn new() -> Self { - Self + Self {} } } } @@ -479,12 +457,6 @@ fn generate_redacted_event_content<'a>( generate_static_event_content_impl(&redacted_ident, kind, true, event_type, ruma_common) }); - let mut event_types = aliases.to_owned(); - event_types.push(event_type); - let event_types = quote! { - [#(#event_types,)*] - }; - Ok(quote! { // this is the non redacted event content's impl #[automatically_derived] @@ -501,32 +473,16 @@ fn generate_redacted_event_content<'a>( #[doc = #doc] #[derive(Clone, Debug, #serde::Deserialize, #serde::Serialize)] #[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)] - pub struct #redacted_ident #redacted_fields + pub struct #redacted_ident { + #( #kept_redacted_fields, )* + } #constructor #redacted_event_content #[automatically_derived] - impl #ruma_common::events::RedactedEventContent for #redacted_ident { - fn empty(ev_type: &str) -> #serde_json::Result { - if !#event_types.contains(&ev_type) { - return Err(#serde::de::Error::custom( - format!("expected event type as one of `{:?}`, found `{}`", #event_types, ev_type) - )); - } - - #redacted_return - } - - fn has_serialize_fields(&self) -> bool { - #has_serialize_fields - } - - fn has_deserialize_fields() -> #ruma_common::events::HasDeserializeFields { - #has_deserialize_fields - } - } + impl #ruma_common::events::RedactedEventContent for #redacted_ident {} #[automatically_derived] impl #ruma_common::events::#sub_trait_name for #redacted_ident {}