From dac493f13a797379b399b13597bb3dee5d597f8f Mon Sep 17 00:00:00 2001
From: Jordan Rose <jrose@signal.org>
Date: Tue, 20 Aug 2024 12:15:31 -0700
Subject: [PATCH] backup: Resolve RecipientIds in reactions, OutgoingSend
 status, and quotes

---
 rust/message-backup/src/backup/chat.rs        | 73 +++++++++++--------
 .../src/backup/chat/contact_message.rs        | 18 +++--
 rust/message-backup/src/backup/chat/quote.rs  | 25 ++++---
 .../src/backup/chat/standard_message.rs       | 20 +++--
 .../src/backup/chat/sticker_message.rs        | 18 +++--
 .../src/backup/chat/voice_message.rs          | 20 +++--
 rust/message-backup/src/backup/serialize.rs   |  2 +-
 7 files changed, 102 insertions(+), 74 deletions(-)

diff --git a/rust/message-backup/src/backup/chat.rs b/rust/message-backup/src/backup/chat.rs
index 5f5733c76..06dd5d3ed 100644
--- a/rust/message-backup/src/backup/chat.rs
+++ b/rust/message-backup/src/backup/chat.rs
@@ -17,7 +17,7 @@ use derive_where::derive_where;
 use crate::backup::chat::chat_style::{ChatStyle, ChatStyleError, CustomColorId};
 use crate::backup::file::{FilePointerError, MessageAttachmentError};
 use crate::backup::frame::RecipientId;
-use crate::backup::method::{Contains, Lookup, Method};
+use crate::backup::method::{Lookup, Method};
 use crate::backup::serialize::{SerializeOrder, UnorderedList};
 use crate::backup::sticker::MessageStickerError;
 use crate::backup::time::{Duration, Timestamp};
@@ -182,7 +182,7 @@ pub struct ChatItemData<M: Method + ReferencedTypes> {
     pub message: ChatItemMessage<M>,
     // This could be Self: Serialize but that just confuses the compiler.
     pub revisions: Vec<ChatItemData<M>>,
-    pub direction: Direction,
+    pub direction: Direction<M::RecipientReference>,
     pub expire_start: Option<Timestamp>,
     pub expires_in: Option<Duration>,
     pub sent_at: Timestamp,
@@ -201,10 +201,10 @@ pub struct ChatItemData<M: Method + ReferencedTypes> {
     M::RecipientReference: PartialEq
 ))]
 pub enum ChatItemMessage<M: Method + ReferencedTypes> {
-    Standard(StandardMessage),
-    Contact(ContactMessage),
-    Voice(VoiceMessage),
-    Sticker(StickerMessage),
+    Standard(StandardMessage<M::RecipientReference>),
+    Contact(ContactMessage<M::RecipientReference>),
+    Voice(VoiceMessage<M::RecipientReference>),
+    Sticker(StickerMessage<M::RecipientReference>),
     RemoteDeleted,
     Update(UpdateMessage<M::RecipientReference>),
     PaymentNotification(PaymentNotification),
@@ -214,21 +214,24 @@ pub enum ChatItemMessage<M: Method + ReferencedTypes> {
 /// Validated version of [`proto::Reaction`].
 #[derive(Debug, serde::Serialize)]
 #[cfg_attr(test, derive(PartialEq, Clone))]
-pub struct Reaction {
+pub struct Reaction<Recipient> {
     pub emoji: String,
     // This field is not generated consistently on all platforms, so we only use it to sort
     // containers of Reactions.
     #[serde(skip)]
     pub sort_order: u64,
-    pub author: RecipientId,
+    #[serde(bound(serialize = "Recipient: serde::Serialize"))]
+    pub author: Recipient,
     pub sent_timestamp: Timestamp,
     pub received_timestamp: Option<Timestamp>,
     _limit_construction_to_module: (),
 }
 
-impl SerializeOrder for Reaction {
+impl<Recipient: SerializeOrder> SerializeOrder for Reaction<Recipient> {
     fn serialize_cmp(&self, other: &Self) -> std::cmp::Ordering {
-        self.sort_order.cmp(&other.sort_order)
+        self.sort_order
+            .cmp(&other.sort_order)
+            .then_with(|| self.author.serialize_cmp(&other.author))
     }
 }
 
@@ -243,21 +246,25 @@ pub enum ReactionError {
 
 #[derive(Debug, serde::Serialize)]
 #[cfg_attr(test, derive(PartialEq))]
-pub enum Direction {
+pub enum Direction<Recipient> {
     Incoming {
         sent: Timestamp,
         received: Timestamp,
         read: bool,
         sealed_sender: bool,
     },
-    Outgoing(UnorderedList<OutgoingSend>),
+    Outgoing(
+        #[serde(bound(serialize = "Recipient: serde::Serialize + SerializeOrder"))]
+        UnorderedList<OutgoingSend<Recipient>>,
+    ),
     Directionless,
 }
 
 #[derive(Debug, serde::Serialize)]
 #[cfg_attr(test, derive(PartialEq, Clone))]
-pub struct OutgoingSend {
-    pub recipient: RecipientId,
+pub struct OutgoingSend<Recipient> {
+    #[serde(bound(serialize = "Recipient: serde::Serialize"))]
+    pub recipient: Recipient,
     pub status: DeliveryStatus,
     pub last_status_update: Timestamp,
 }
@@ -483,12 +490,14 @@ impl<
     }
 }
 
-impl<R: Contains<RecipientId>> TryFromWith<proto::chat_item::DirectionalDetails, R> for Direction {
+impl<R: Clone, C: Lookup<RecipientId, R>> TryFromWith<proto::chat_item::DirectionalDetails, C>
+    for Direction<R>
+{
     type Error = ChatItemError;
 
     fn try_from_with(
         item: proto::chat_item::DirectionalDetails,
-        context: &R,
+        context: &C,
     ) -> Result<Self, Self::Error> {
         use proto::chat_item::*;
         match item {
@@ -525,10 +534,10 @@ impl<R: Contains<RecipientId>> TryFromWith<proto::chat_item::DirectionalDetails,
         }
     }
 }
-impl<R: Contains<RecipientId>> TryFromWith<proto::SendStatus, R> for OutgoingSend {
+impl<R: Clone, C: Lookup<RecipientId, R>> TryFromWith<proto::SendStatus, C> for OutgoingSend<R> {
     type Error = OutgoingSendError;
 
-    fn try_from_with(item: proto::SendStatus, context: &R) -> Result<Self, Self::Error> {
+    fn try_from_with(item: proto::SendStatus, context: &C) -> Result<Self, Self::Error> {
         let proto::SendStatus {
             recipientId,
             timestamp,
@@ -536,11 +545,10 @@ impl<R: Contains<RecipientId>> TryFromWith<proto::SendStatus, R> for OutgoingSen
             special_fields: _,
         } = item;
 
-        let recipient = RecipientId(recipientId);
-
-        if !context.contains(&recipient) {
-            return Err(OutgoingSendError::UnknownRecipient(recipient));
-        }
+        let recipient_id = RecipientId(recipientId);
+        let Some(recipient) = context.lookup(&recipient_id).cloned() else {
+            return Err(OutgoingSendError::UnknownRecipient(recipient_id));
+        };
 
         let Some(status) = deliveryStatus else {
             return Err(OutgoingSendError::SendStatusMissing);
@@ -648,10 +656,10 @@ impl<
     }
 }
 
-impl<R: Contains<RecipientId>> TryFromWith<proto::Reaction, R> for Reaction {
+impl<R: Clone, C: Lookup<RecipientId, R>> TryFromWith<proto::Reaction, C> for Reaction<R> {
     type Error = ReactionError;
 
-    fn try_from_with(item: proto::Reaction, context: &R) -> Result<Self, Self::Error> {
+    fn try_from_with(item: proto::Reaction, context: &C) -> Result<Self, Self::Error> {
         let proto::Reaction {
             authorId,
             sentTimestamp,
@@ -665,10 +673,10 @@ impl<R: Contains<RecipientId>> TryFromWith<proto::Reaction, R> for Reaction {
             return Err(ReactionError::EmptyEmoji);
         }
 
-        let author = RecipientId(authorId);
-        if !context.contains(&author) {
-            return Err(ReactionError::AuthorNotFound(author));
-        }
+        let author_id = RecipientId(authorId);
+        let Some(author) = context.lookup(&author_id).cloned() else {
+            return Err(ReactionError::AuthorNotFound(author_id));
+        };
 
         let sent_timestamp = Timestamp::from_millis(sentTimestamp, "Reaction.sentTimestamp");
         let received_timestamp = receivedTimestamp
@@ -696,6 +704,7 @@ mod test {
 
     use crate::backup::chat::testutil::TestContext;
     use crate::backup::method::Store;
+    use crate::backup::recipient::FullRecipientData;
     use crate::backup::time::testutil::MillisecondsSinceEpoch;
     use crate::backup::Purpose;
 
@@ -737,12 +746,12 @@ mod test {
         }
     }
 
-    impl Reaction {
+    impl Reaction<FullRecipientData> {
         pub(crate) fn from_proto_test_data() -> Self {
             Self {
                 emoji: "📲".to_string(),
                 sort_order: 3,
-                author: RecipientId(proto::Recipient::TEST_ID),
+                author: TestContext::test_recipient().clone(),
                 sent_timestamp: Timestamp::test_value(),
                 received_timestamp: Some(Timestamp::test_value()),
                 _limit_construction_to_module: (),
@@ -898,7 +907,7 @@ mod test {
 
         reaction
             .try_into_with(&TestContext::default())
-            .map(|_: Reaction| ())
+            .map(|_: Reaction<FullRecipientData>| ())
     }
 
     #[test_case(Purpose::DeviceTransfer, 3600, Ok(()))]
diff --git a/rust/message-backup/src/backup/chat/contact_message.rs b/rust/message-backup/src/backup/chat/contact_message.rs
index cef2fd778..ac39abbca 100644
--- a/rust/message-backup/src/backup/chat/contact_message.rs
+++ b/rust/message-backup/src/backup/chat/contact_message.rs
@@ -7,17 +7,18 @@ use protobuf::EnumOrUnknown;
 use crate::backup::chat::{ChatItemError, Reaction};
 use crate::backup::file::{FilePointer, FilePointerError};
 use crate::backup::frame::RecipientId;
-use crate::backup::method::Contains;
-use crate::backup::serialize::UnorderedList;
+use crate::backup::method::Lookup;
+use crate::backup::serialize::{SerializeOrder, UnorderedList};
 use crate::backup::{TryFromWith, TryIntoWith as _};
 use crate::proto::backup as proto;
 
 /// Validated version of [`proto::ContactMessage`].
 #[derive(Debug, serde::Serialize)]
 #[cfg_attr(test, derive(PartialEq))]
-pub struct ContactMessage {
+pub struct ContactMessage<Recipient> {
     pub contacts: Vec<ContactAttachment>,
-    pub reactions: UnorderedList<Reaction>,
+    #[serde(bound(serialize = "Recipient: serde::Serialize + SerializeOrder"))]
+    pub reactions: UnorderedList<Reaction<Recipient>>,
     _limit_construction_to_module: (),
 }
 
@@ -44,10 +45,12 @@ pub enum ContactAttachmentError {
     Avatar(FilePointerError),
 }
 
-impl<R: Contains<RecipientId>> TryFromWith<proto::ContactMessage, R> for ContactMessage {
+impl<R: Clone, C: Lookup<RecipientId, R>> TryFromWith<proto::ContactMessage, C>
+    for ContactMessage<R>
+{
     type Error = ChatItemError;
 
-    fn try_from_with(item: proto::ContactMessage, context: &R) -> Result<Self, Self::Error> {
+    fn try_from_with(item: proto::ContactMessage, context: &C) -> Result<Self, Self::Error> {
         let proto::ContactMessage {
             reactions,
             contact,
@@ -171,6 +174,7 @@ mod test {
 
     use crate::backup::chat::testutil::TestContext;
     use crate::backup::chat::ReactionError;
+    use crate::backup::recipient::FullRecipientData;
 
     use super::*;
 
@@ -238,6 +242,6 @@ mod test {
 
         message
             .try_into_with(&TestContext::default())
-            .map(|_: ContactMessage| ())
+            .map(|_: ContactMessage<FullRecipientData>| ())
     }
 }
diff --git a/rust/message-backup/src/backup/chat/quote.rs b/rust/message-backup/src/backup/chat/quote.rs
index 66f4e3ed4..8088d3725 100644
--- a/rust/message-backup/src/backup/chat/quote.rs
+++ b/rust/message-backup/src/backup/chat/quote.rs
@@ -6,7 +6,7 @@
 use crate::backup::chat::text::{MessageText, TextError};
 use crate::backup::file::{MessageAttachment, MessageAttachmentError};
 use crate::backup::frame::RecipientId;
-use crate::backup::method::Contains;
+use crate::backup::method::Lookup;
 use crate::backup::time::Timestamp;
 use crate::backup::TryFromWith;
 use crate::proto::backup as proto;
@@ -14,8 +14,9 @@ use crate::proto::backup as proto;
 /// Validated version of [`proto::Quote`]
 #[derive(Debug, serde::Serialize)]
 #[cfg_attr(test, derive(PartialEq))]
-pub struct Quote {
-    pub author: RecipientId,
+pub struct Quote<Recipient> {
+    #[serde(bound(serialize = "Recipient: serde::Serialize"))]
+    pub author: Recipient,
     pub quote_type: QuoteType,
     pub target_sent_timestamp: Option<Timestamp>,
     pub attachments: Vec<QuotedAttachment>,
@@ -56,10 +57,10 @@ pub enum QuoteError {
     AttachmentThumbnailWrongFlag(proto::message_attachment::Flag),
 }
 
-impl<R: Contains<RecipientId>> TryFromWith<proto::Quote, R> for Quote {
+impl<R: Clone, C: Lookup<RecipientId, R>> TryFromWith<proto::Quote, C> for Quote<R> {
     type Error = QuoteError;
 
-    fn try_from_with(item: proto::Quote, context: &R) -> Result<Self, Self::Error> {
+    fn try_from_with(item: proto::Quote, context: &C) -> Result<Self, Self::Error> {
         let proto::Quote {
             authorId,
             type_,
@@ -69,10 +70,10 @@ impl<R: Contains<RecipientId>> TryFromWith<proto::Quote, R> for Quote {
             special_fields: _,
         } = item;
 
-        let author = RecipientId(authorId);
-        if !context.contains(&author) {
-            return Err(QuoteError::AuthorNotFound(author));
-        }
+        let author_id = RecipientId(authorId);
+        let Some(author) = context.lookup(&author_id).cloned() else {
+            return Err(QuoteError::AuthorNotFound(author_id));
+        };
 
         let target_sent_timestamp = targetSentTimestamp
             .map(|timestamp| Timestamp::from_millis(timestamp, "Quote.targetSentTimestamp"));
@@ -140,6 +141,8 @@ impl TryFrom<proto::quote::QuotedAttachment> for QuotedAttachment {
 mod test {
     use test_case::test_case;
 
+    use crate::backup::chat::testutil::TestContext;
+    use crate::backup::recipient::FullRecipientData;
     use crate::backup::time::testutil::MillisecondsSinceEpoch;
 
     use super::*;
@@ -202,11 +205,11 @@ mod test {
         }
     }
 
-    impl Quote {
+    impl Quote<FullRecipientData> {
         pub(crate) fn from_proto_test_data() -> Self {
             Self {
                 text: Some(MessageText::from_proto_test_data()),
-                author: RecipientId(proto::Recipient::TEST_ID),
+                author: TestContext::test_recipient().clone(),
                 quote_type: QuoteType::Normal,
                 target_sent_timestamp: Some(Timestamp::test_value()),
                 attachments: vec![QuotedAttachment::from_proto_test_data()],
diff --git a/rust/message-backup/src/backup/chat/standard_message.rs b/rust/message-backup/src/backup/chat/standard_message.rs
index c94d78540..e0adc9193 100644
--- a/rust/message-backup/src/backup/chat/standard_message.rs
+++ b/rust/message-backup/src/backup/chat/standard_message.rs
@@ -8,28 +8,31 @@ use crate::backup::chat::text::MessageText;
 use crate::backup::chat::{ChatItemError, Reaction};
 use crate::backup::file::{FilePointer, MessageAttachment};
 use crate::backup::frame::RecipientId;
-use crate::backup::method::Contains;
-use crate::backup::serialize::UnorderedList;
+use crate::backup::method::Lookup;
+use crate::backup::serialize::{SerializeOrder, UnorderedList};
 use crate::backup::{TryFromWith, TryIntoWith as _};
 use crate::proto::backup as proto;
 
 /// Validated version of [`proto::StandardMessage`].
 #[derive(Debug, serde::Serialize)]
 #[cfg_attr(test, derive(PartialEq))]
-pub struct StandardMessage {
+pub struct StandardMessage<Recipient> {
     pub text: Option<MessageText>,
-    pub quote: Option<Quote>,
+    pub quote: Option<Quote<Recipient>>,
     pub attachments: Vec<MessageAttachment>,
-    pub reactions: UnorderedList<Reaction>,
+    #[serde(bound(serialize = "Recipient: serde::Serialize + SerializeOrder"))]
+    pub reactions: UnorderedList<Reaction<Recipient>>,
     pub link_previews: Vec<LinkPreview>,
     pub long_text: Option<FilePointer>,
     _limit_construction_to_module: (),
 }
 
-impl<R: Contains<RecipientId>> TryFromWith<proto::StandardMessage, R> for StandardMessage {
+impl<R: Clone, C: Lookup<RecipientId, R>> TryFromWith<proto::StandardMessage, C>
+    for StandardMessage<R>
+{
     type Error = ChatItemError;
 
-    fn try_from_with(item: proto::StandardMessage, context: &R) -> Result<Self, Self::Error> {
+    fn try_from_with(item: proto::StandardMessage, context: &C) -> Result<Self, Self::Error> {
         let proto::StandardMessage {
             text,
             quote,
@@ -83,6 +86,7 @@ impl<R: Contains<RecipientId>> TryFromWith<proto::StandardMessage, R> for Standa
 #[cfg(test)]
 mod test {
     use crate::backup::chat::testutil::TestContext;
+    use crate::backup::recipient::FullRecipientData;
     use crate::backup::time::{Duration, Timestamp};
 
     use super::*;
@@ -110,7 +114,7 @@ mod test {
         }
     }
 
-    impl StandardMessage {
+    impl StandardMessage<FullRecipientData> {
         pub(crate) fn from_proto_test_data() -> Self {
             Self {
                 text: Some(MessageText::from_proto_test_data()),
diff --git a/rust/message-backup/src/backup/chat/sticker_message.rs b/rust/message-backup/src/backup/chat/sticker_message.rs
index 76f0a926b..556984495 100644
--- a/rust/message-backup/src/backup/chat/sticker_message.rs
+++ b/rust/message-backup/src/backup/chat/sticker_message.rs
@@ -4,8 +4,8 @@
 
 use crate::backup::chat::{ChatItemError, Reaction};
 use crate::backup::frame::RecipientId;
-use crate::backup::method::Contains;
-use crate::backup::serialize::UnorderedList;
+use crate::backup::method::Lookup;
+use crate::backup::serialize::{SerializeOrder, UnorderedList};
 use crate::backup::sticker::MessageSticker;
 use crate::backup::{TryFromWith, TryIntoWith as _};
 use crate::proto::backup as proto;
@@ -13,16 +13,19 @@ use crate::proto::backup as proto;
 /// Validated version of [`proto::StickerMessage`].
 #[derive(Debug, serde::Serialize)]
 #[cfg_attr(test, derive(PartialEq))]
-pub struct StickerMessage {
-    pub reactions: UnorderedList<Reaction>,
+pub struct StickerMessage<Recipient> {
+    #[serde(bound(serialize = "Recipient: serde::Serialize + SerializeOrder"))]
+    pub reactions: UnorderedList<Reaction<Recipient>>,
     pub sticker: MessageSticker,
     _limit_construction_to_module: (),
 }
 
-impl<R: Contains<RecipientId>> TryFromWith<proto::StickerMessage, R> for StickerMessage {
+impl<R: Clone, C: Lookup<RecipientId, R>> TryFromWith<proto::StickerMessage, C>
+    for StickerMessage<R>
+{
     type Error = ChatItemError;
 
-    fn try_from_with(item: proto::StickerMessage, context: &R) -> Result<Self, Self::Error> {
+    fn try_from_with(item: proto::StickerMessage, context: &C) -> Result<Self, Self::Error> {
         let proto::StickerMessage {
             reactions,
             sticker,
@@ -53,6 +56,7 @@ mod test {
 
     use crate::backup::chat::testutil::TestContext;
     use crate::backup::chat::ReactionError;
+    use crate::backup::recipient::FullRecipientData;
 
     use super::*;
 
@@ -74,6 +78,6 @@ mod test {
 
         message
             .try_into_with(&TestContext::default())
-            .map(|_: StickerMessage| ())
+            .map(|_: StickerMessage<FullRecipientData>| ())
     }
 }
diff --git a/rust/message-backup/src/backup/chat/voice_message.rs b/rust/message-backup/src/backup/chat/voice_message.rs
index 9a902496d..c5fa01502 100644
--- a/rust/message-backup/src/backup/chat/voice_message.rs
+++ b/rust/message-backup/src/backup/chat/voice_message.rs
@@ -6,17 +6,18 @@ use crate::backup::chat::quote::{Quote, QuoteError};
 use crate::backup::chat::{Reaction, ReactionError};
 use crate::backup::file::{MessageAttachment, MessageAttachmentError};
 use crate::backup::frame::RecipientId;
-use crate::backup::method::Contains;
-use crate::backup::serialize::UnorderedList;
+use crate::backup::method::Lookup;
+use crate::backup::serialize::{SerializeOrder, UnorderedList};
 use crate::backup::{TryFromWith, TryIntoWith as _};
 use crate::proto::backup as proto;
 
 /// Validated version of a voice message [`proto::StandardMessage`].
 #[derive(Debug, serde::Serialize)]
 #[cfg_attr(test, derive(PartialEq))]
-pub struct VoiceMessage {
-    pub quote: Option<Quote>,
-    pub reactions: UnorderedList<Reaction>,
+pub struct VoiceMessage<Recipient> {
+    pub quote: Option<Quote<Recipient>>,
+    #[serde(bound(serialize = "Recipient: serde::Serialize + SerializeOrder"))]
+    pub reactions: UnorderedList<Reaction<Recipient>>,
     pub attachment: MessageAttachment,
     _limit_construction_to_module: (),
 }
@@ -38,10 +39,12 @@ pub enum VoiceMessageError {
     Reaction(#[from] ReactionError),
 }
 
-impl<R: Contains<RecipientId>> TryFromWith<proto::StandardMessage, R> for VoiceMessage {
+impl<R: Clone, C: Lookup<RecipientId, R>> TryFromWith<proto::StandardMessage, C>
+    for VoiceMessage<R>
+{
     type Error = VoiceMessageError;
 
-    fn try_from_with(item: proto::StandardMessage, context: &R) -> Result<Self, Self::Error> {
+    fn try_from_with(item: proto::StandardMessage, context: &C) -> Result<Self, Self::Error> {
         let proto::StandardMessage {
             quote,
             reactions,
@@ -92,6 +95,7 @@ mod test {
     use test_case::test_case;
 
     use crate::backup::chat::testutil::TestContext;
+    use crate::backup::recipient::FullRecipientData;
 
     use super::*;
 
@@ -120,6 +124,6 @@ mod test {
 
         message
             .try_into_with(&TestContext::default())
-            .map(|_: VoiceMessage| ())
+            .map(|_: VoiceMessage<FullRecipientData>| ())
     }
 }
diff --git a/rust/message-backup/src/backup/serialize.rs b/rust/message-backup/src/backup/serialize.rs
index 187a72cb3..0ad02df8f 100644
--- a/rust/message-backup/src/backup/serialize.rs
+++ b/rust/message-backup/src/backup/serialize.rs
@@ -217,7 +217,7 @@ impl SerializeOrder for TextRange {
     }
 }
 
-impl SerializeOrder for OutgoingSend {
+impl<R: SerializeOrder> SerializeOrder for OutgoingSend<R> {
     fn serialize_cmp(&self, other: &Self) -> std::cmp::Ordering {
         self.recipient
             .serialize_cmp(&other.recipient)