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.

Sign up for GitHub

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

History Compatibility #755

Closed
wants to merge 1 commit into from
Closed

History Compatibility #755

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
198 changes: 194 additions & 4 deletions data/src/message.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ static URL_REGEX: Lazy<Regex> = Lazy::new(|| {
.unwrap()
});

pub type Channel = String;

pub(crate) mod broadcast;
pub mod formatting;
pub mod source;
Expand Down Expand Up @@ -107,7 +105,7 @@ impl From<Encoded> for proto::Message {
}
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[derive(Debug, Clone, PartialEq, Eq)]
Comment on lines -110 to +108
Copy link
Member

Choose a reason for hiding this comment

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

My understanding is the only breakage is we renamed Query.nick to Query.query. We can use #[serde(alias)] to provide upgrade compatability from < 2025.1 to >= 2025.1

Copy link
Member

Choose a reason for hiding this comment

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

Shit, message::Source::Action is also breaking as we added a payload to it :/

Copy link
Member

Choose a reason for hiding this comment

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

We may want to do a bigger, more comprehensive update that can accomplish the following:

  • Safely upgrades from 2025.1
  • Is more easily upgradable in the future without breakage (no unit variants, only struct variants so we can always add fields)

We need to separate the serialization format from the data format we use at runtime. Having them be the same makes it way too easy for breakage to occur. Having them separate and a simple From / Into conversion makes it super easy to see what needs to change in a compatible way w/ the serialization format, when the data format changes.

pub enum Target {
Server {
source: Source,
Expand All @@ -128,6 +126,182 @@ pub enum Target {
},
}

impl Serialize for Target {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
#[derive(Serialize)]
enum Data<'a> {
Server {
source: &'a Source,
},
Channel {
target_channel: &'a target::Channel,
source: &'a Source,
channel: &'a str,
prefix: Option<&'a char>,
},
Query {
target_query: &'a target::Query,
source: &'a Source,
nick: Cow<'a, Nick>,
},
Logs,
Highlights {
server: &'a Server,
target_channel: &'a target::Channel,
source: &'a Source,
channel: &'a str,
},
}

match &self {
Target::Server { source } => Data::Server { source },
Target::Channel { channel, source } => Data::Channel {
target_channel: channel,
source,
channel: channel.as_str(),
prefix: channel.prefixes().first(),
},
Target::Query { query, source } => Data::Query {
target_query: query,
source,
nick: Cow::Owned(Nick::from(query.as_str())),
},
Target::Logs => Data::Logs,
Target::Highlights {
server,
channel,
source,
} => Data::Highlights {
server,
target_channel: channel,
source,
channel: channel.as_str(),
},
}
.serialize(serializer)
}
}

impl<'de> Deserialize<'de> for Target {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
#[derive(Deserialize)]
enum Data {
Server {
source: Source,
},
Channel {
#[serde(default, deserialize_with = "fail_as_none")]
target_channel: Option<target::Channel>,
source: Source,
#[serde(default, deserialize_with = "fail_as_none")]
channel: Option<String>,
#[serde(default, deserialize_with = "fail_as_none")]
prefix: Option<char>,
},
Query {
#[serde(default, deserialize_with = "fail_as_none")]
target_query: Option<target::Query>,
source: Source,
#[serde(default, deserialize_with = "fail_as_none")]
nick: Option<Nick>,
},
Logs,
Highlights {
server: Server,
#[serde(default, deserialize_with = "fail_as_none")]
target_channel: Option<target::Channel>,
source: Source,
#[serde(default, deserialize_with = "fail_as_none")]
channel: Option<String>,
},
}

let data = Data::deserialize(deserializer)?;

let target = match data {
Data::Server { source } => Target::Server { source },
Data::Channel {
target_channel,
source,
channel,
prefix,
} => {
if let Some(target_channel) = target_channel {
Target::Channel {
channel: target_channel,
source,
}
} else {
let channel = channel
.ok_or(serde::de::Error::custom("unable to parse channel target"))?;

let channel = if let Some(prefix) = prefix {
let channel = String::from(prefix) + &channel;

target::Channel::from_str(&channel, isupport::CaseMap::default())
} else {
target::Channel::from_str(&channel, isupport::CaseMap::default())
};

Target::Channel { channel, source }
}
}
Data::Query {
target_query,
source,
nick,
} => {
if let Some(target_query) = target_query {
Target::Query {
query: target_query,
source,
}
} else {
let nick =
nick.ok_or(serde::de::Error::custom("unable to parse query target"))?;
let user = User::from(nick);
Target::Query {
query: target::Query::from_user(&user, isupport::CaseMap::default()),
source,
}
}
}
Data::Logs => Target::Logs,
Data::Highlights {
server,
target_channel,
source,
channel,
} => {
if let Some(target_channel) = target_channel {
Target::Highlights {
server,
channel: target_channel,
source,
}
} else {
let channel = channel.ok_or(serde::de::Error::custom(
"unable to parse highlights target",
))?;
Target::Highlights {
server,
channel: target::Channel::from_str(&channel, isupport::CaseMap::default()),
source,
}
}
}
};

Ok(target)
}
}

impl Target {
pub fn prefixes(&self) -> Option<&[char]> {
match self {
Expand Down Expand Up @@ -443,7 +617,7 @@ impl<'de> Deserialize<'de> for Message {
// Old field before we had fragments
text: Option<String>,
id: Option<String>,
#[serde(default)]
#[serde(default, deserialize_with = "fail_as_empty_set")]
hidden_urls: HashSet<url::Url>,
// New field, optional for upgrade compatability
#[serde(default, deserialize_with = "fail_as_none")]
Expand Down Expand Up @@ -1417,6 +1591,22 @@ where
Ok(Option::<T>::deserialize(intermediate).unwrap_or_default())
}

fn fail_as_empty_set<'de, T, D>(deserializer: D) -> Result<HashSet<T>, D::Error>
where
T: Deserialize<'de>,
T: std::cmp::Eq,
T: std::hash::Hash,
D: Deserializer<'de>,
{
// We must fully consume valid json otherwise the error leaves the
// deserializer in an invalid state and it'll still fail
//
// This assumes we always use a json format
let intermediate = serde_json::Value::deserialize(deserializer)?;

Ok(HashSet::<T>::deserialize(intermediate).unwrap_or_default())
}

#[derive(Debug, Clone, Default, Deserialize, Serialize)]
pub struct MessageReferences {
pub timestamp: DateTime<Utc>,
Expand Down