Skip to content
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

json deserialization for legacy reactions #1488

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions xmtp_content_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ license.workspace = true
thiserror = { workspace = true }
prost = { workspace = true, features = ["prost-derive"] }
rand = { workspace = true }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
tracing.workspace = true

# XMTP/Local
xmtp_proto = { workspace = true, features = ["convert"] }
Expand Down
61 changes: 61 additions & 0 deletions xmtp_content_types/src/reaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::collections::HashMap;
use crate::{CodecError, ContentCodec};
use prost::Message;

use serde::{Deserialize, Serialize};
use xmtp_proto::xmtp::mls::message_contents::{
content_types::ReactionV2, ContentTypeId, EncodedContent,
};
Expand Down Expand Up @@ -47,6 +48,42 @@ impl ContentCodec<ReactionV2> for ReactionCodec {
}
}

// JSON format for legacy reaction is defined here: https://github.com/xmtp/xmtp-js/blob/main/content-types/content-type-reaction/src/Reaction.ts
#[derive(Debug, Serialize, Deserialize)]
pub struct LegacyReaction {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't shoe-horn the legacy reaction into the new reaction type? I thought the data was about the same

Copy link
Contributor Author

@cameronvoell cameronvoell Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original type had arbitrary String for reaction action and reaction schema (js sdk used union type to restrict it), I updated to enum in the new proto type to be more precise so we wouldnt need to do this type of conversion across all the SDKs: https://github.com/xmtp/xmtp-android/blob/51c7b170d53f73d32d3a1d33b0d170d156183e43/library/src/main/java/org/xmtp/android/library/codecs/ReactionCodec.kt#L40-L55

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh. Too bad.

Agree the new format is nicer.

/// The message ID for the message that is being reacted to
pub reference: String,
/// The inbox ID of the user who sent the message that is being reacted to
#[serde(rename = "referenceInboxId", skip_serializing_if = "Option::is_none")]
pub reference_inbox_id: Option<String>,
/// The action of the reaction ("added" or "removed")
pub action: String,
/// The content of the reaction
pub content: String,
/// The schema of the content ("unicode", "shortcode", or "custom")
pub schema: String,
}

impl LegacyReaction {
pub fn decode(content: &[u8]) -> Option<LegacyReaction> {
// Try to decode the content as UTF-8 string first
if let Ok(decoded_content) = String::from_utf8(content.to_vec()) {
tracing::info!(
"attempting legacy json deserialization: {}",
decoded_content
);
// Try parsing as canonical JSON format
if let Ok(reaction) = serde_json::from_str::<LegacyReaction>(&decoded_content) {
return Some(reaction);
}
tracing::error!("legacy json deserialization failed");
} else {
tracing::error!("utf-8 deserialization failed");
}
None
}
}

#[cfg(test)]
pub(crate) mod tests {
#[cfg(target_arch = "wasm32")]
Expand All @@ -56,6 +93,7 @@ pub(crate) mod tests {
ReactionAction, ReactionSchema, ReactionV2,
};

use serde_json::json;
use xmtp_common::rand_string;

use super::*;
Expand All @@ -80,4 +118,27 @@ pub(crate) mod tests {
assert_eq!(decoded.content, "👍".to_string());
assert_eq!(decoded.schema, ReactionSchema::Unicode as i32);
}

#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
#[cfg_attr(not(target_arch = "wasm32"), test)]
fn test_legacy_reaction_deserialization() {
let reference = "0123456789abcdef";
let legacy_json = json!({
"reference": reference,
"referenceInboxId": "some_inbox_id",
"action": "added",
"content": "👍",
"schema": "unicode"
});

let content = legacy_json.to_string().into_bytes();
let decoded_reference: String = LegacyReaction::decode(&content).unwrap().reference;

assert_eq!(decoded_reference, reference);

// Test invalid JSON
let invalid_content = b"invalid json";
let failed_decode = LegacyReaction::decode(invalid_content);
assert!(failed_decode.is_none());
}
}
17 changes: 9 additions & 8 deletions xmtp_mls/src/groups/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use openmls_traits::OpenMlsProvider;
use prost::Message;
use thiserror::Error;
use tokio::sync::Mutex;
use xmtp_content_types::reaction::ReactionCodec;
use xmtp_content_types::reaction::{LegacyReaction, ReactionCodec};

use self::device_sync::DeviceSyncError;
pub use self::group_permissions::PreconfiguredPolicies;
Expand Down Expand Up @@ -348,13 +348,12 @@ impl TryFrom<EncodedContent> for QueryableContentFields {
content_type_id.version_major,
) {
(ReactionCodec::TYPE_ID, major) if major >= 2 => {
let reaction = ReactionV2::decode(content.content.as_slice())?;
hex::decode(reaction.reference).ok()
}
(ReactionCodec::TYPE_ID, _) => {
// TODO: Implement JSON deserialization for legacy reaction format
None
ReactionV2::decode(content.content.as_slice())
.ok()
.and_then(|reaction| hex::decode(reaction.reference).ok())
}
(ReactionCodec::TYPE_ID, _) => LegacyReaction::decode(&content.content)
.and_then(|legacy_reaction| hex::decode(legacy_reaction.reference).ok()),
_ => None,
};

Expand Down Expand Up @@ -796,7 +795,9 @@ impl<ScopedClient: ScopedGroupClient> MlsGroup<ScopedClient> {
fn extract_queryable_content_fields(message: &[u8]) -> QueryableContentFields {
// Return early with default if decoding fails or type is missing
EncodedContent::decode(message)
.inspect_err(|e| tracing::debug!("Failed to decode message as EncodedContent: {}", e))
.inspect_err(|_| {
tracing::debug!("No queryable content fields, msg not formatted as encoded content")
})
.and_then(|content| {
QueryableContentFields::try_from(content).inspect_err(|e| {
tracing::debug!(
Expand Down
Loading