Expand Echo Search (Take Two) #732
Conversation
andymandias
force-pushed
the
fix/expanded-echo-search
branch
from
a273fea to
ce27d8d
Compare
data/src/message.rs
Outdated
| #[derive(Debug, Clone, Copy)] | ||
| pub enum Direction { | ||
| Sent, | ||
| Received, | ||
| Received { is_echo: bool }, | ||
| } | ||
|
|
||
| impl Serialize for Direction { | ||
| fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> | ||
| where | ||
| S: serde::Serializer, | ||
| { | ||
| #[derive(Serialize)] | ||
| enum Data { | ||
| Sent, | ||
| Received, | ||
| } | ||
|
|
||
| match self { | ||
| Direction::Sent => Data::Sent, | ||
| Direction::Received { .. } => Data::Received, | ||
| } | ||
| .serialize(serializer) | ||
| } | ||
| } | ||
|
|
||
| impl<'de> Deserialize<'de> for Direction { | ||
| fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> | ||
| where | ||
| D: serde::Deserializer<'de>, | ||
| { | ||
| #[derive(Deserialize)] | ||
| enum Data { | ||
| Sent, | ||
| Received, | ||
| } | ||
|
|
||
| let data = Data::deserialize(deserializer)?; | ||
|
|
||
| Ok(match data { | ||
| Data::Sent => Direction::Sent, | ||
| Data::Received => Direction::Received { is_echo: false }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
I don't love that we're discarding this from state, so it's effectively a run-time only variable that's discarded upon restart. There's nothing preventing someone from depending on this field in more complex ways that then breaks due to this.
As much as I'd love to add this to Direction, it doesn't need to be encoded on it. Let's just add a new is_echo field to the message and this is backwards compatible since older clients will just ignore that in the json.
There was a problem hiding this comment.
Was hoping there would be some serde incantation that would preserve is_echo only when Direction::Received, but I couldn't figure it out. Your rationale for the new field makes sense to me → updated (& tested) the implementation.
…bbering existing history).
andymandias
force-pushed
the
fix/expanded-echo-search
branch
from
ce27d8d to
8a029d0
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
data/src/message.rs
Outdated
| received_at, | ||
| server_time, | ||
| server_time: Utc::now(), |
data/src/message.rs
Outdated
| @@ -456,6 +472,8 @@ impl<'de> Deserialize<'de> for Message { | |||
| Content::Plain("".to_string()) | |||
| }; | |||
|
|
|||
| let is_echo = is_echo.unwrap_or(false); | |||
Ah shoot, didn't catch rebase adding that. |
Increases the search time interval when inserting an echo (message sent by the user received from the server) into message history. Unlike the previous implementation (#714), existing history is properly deserialized.