diff --git a/Cargo.lock b/Cargo.lock index 4eb629c4c..4c7b3ed81 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -129,6 +129,7 @@ dependencies = [ "num_enum", "once_cell", "percent-encoding-iri", + "phf 0.11.2", "pin-project", "prettyplease 0.2.9", "prost", diff --git a/rslib/Cargo.toml b/rslib/Cargo.toml index 67be8d3b3..03e8fbe71 100644 --- a/rslib/Cargo.toml +++ b/rslib/Cargo.toml @@ -72,6 +72,7 @@ num_cpus.workspace = true num_enum.workspace = true once_cell.workspace = true percent-encoding-iri.workspace = true +phf.workspace = true pin-project.workspace = true prost.workspace = true pulldown-cmark.workspace = true diff --git a/rslib/src/deckconfig/schema11.rs b/rslib/src/deckconfig/schema11.rs index 856fbbe81..d9b5f876f 100644 --- a/rslib/src/deckconfig/schema11.rs +++ b/rslib/src/deckconfig/schema11.rs @@ -3,6 +3,8 @@ use std::collections::HashMap; +use phf::phf_set; +use phf::Set; use serde::Deserialize as DeTrait; use serde::Deserialize; use serde::Deserializer; @@ -337,15 +339,19 @@ impl From for DeckConfSchema11 { if let Some(new) = top_other.remove("new") { let val: HashMap = serde_json::from_value(new).unwrap_or_default(); new_other = val; + new_other.retain(|k, _v| !RESERVED_DECKCONF_NEW_KEYS.contains(k)) } if let Some(rev) = top_other.remove("rev") { let val: HashMap = serde_json::from_value(rev).unwrap_or_default(); rev_other = val; + rev_other.retain(|k, _v| !RESERVED_DECKCONF_REV_KEYS.contains(k)) } if let Some(lapse) = top_other.remove("lapse") { let val: HashMap = serde_json::from_value(lapse).unwrap_or_default(); lapse_other = val; + lapse_other.retain(|k, _v| !RESERVED_DECKCONF_LAPSE_KEYS.contains(k)) } + top_other.retain(|k, _v| !RESERVED_DECKCONF_KEYS.contains(k)); } let i = c.inner; let new_order = i.new_card_insert_order(); @@ -407,13 +413,62 @@ impl From for DeckConfSchema11 { } } +static RESERVED_DECKCONF_KEYS: Set<&'static str> = phf_set! { + "id", + "newSortOrder", + "replayq", + "newPerDayMinimum", + "usn", + "autoplay", + "dyn", + "maxTaken", + "reviewOrder", + "buryInterdayLearning", + "newMix", + "mod", + "timer", + "name", + "interdayLearningMix", + "newGatherPriority" +}; + +static RESERVED_DECKCONF_NEW_KEYS: Set<&'static str> = phf_set! { + "order", "delays", "bury", "perDay", "initialFactor", "ints" +}; + +static RESERVED_DECKCONF_REV_KEYS: Set<&'static str> = phf_set! { + "maxIvl", "hardFactor", "ease4", "ivlFct", "perDay", "bury" +}; + +static RESERVED_DECKCONF_LAPSE_KEYS: Set<&'static str> = phf_set! { + "leechFails", "mult", "leechAction", "delays", "minInt" +}; + #[cfg(test)] mod test { + use itertools::Itertools; use serde::de::IntoDeserializer; use serde_json::json; use serde_json::Value; use super::*; + use crate::prelude::*; + + #[test] + fn all_reserved_fields_are_removed() -> Result<()> { + let key_source = DeckConfSchema11::default(); + let mut config = DeckConfig::default(); + let empty: &[&String] = &[]; + + config.inner.other = serde_json::to_vec(&key_source)?; + let s11 = DeckConfSchema11::from(config); + assert_eq!(&s11.other.keys().collect_vec(), empty); + assert_eq!(&s11.new.other.keys().collect_vec(), empty); + assert_eq!(&s11.rev.other.keys().collect_vec(), empty); + assert_eq!(&s11.lapse.other.keys().collect_vec(), empty); + + Ok(()) + } #[test] fn new_intervals() { diff --git a/rslib/src/decks/schema11.rs b/rslib/src/decks/schema11.rs index 49fc07dd0..a598cceeb 100644 --- a/rslib/src/decks/schema11.rs +++ b/rslib/src/decks/schema11.rs @@ -4,6 +4,8 @@ use std::collections::HashMap; use anki_proto::decks::deck::normal::DayLimit; +use phf::phf_set; +use phf::Set; use serde::Deserialize; use serde::Serialize; use serde_json::Value; @@ -13,6 +15,7 @@ use super::DeckCommon; use super::FilteredDeck; use super::FilteredSearchTerm; use super::NormalDeck; +use crate::notetype::schema11::parse_other_fields; use crate::prelude::*; use crate::serde::default_on_invalid; use crate::serde::deserialize_bool_from_anything; @@ -201,13 +204,6 @@ impl DeckSchema11 { } } - // pub(crate) fn common_mut(&mut self) -> &mut DeckCommon { - // match self { - // Deck::Normal(d) => &mut d.common, - // Deck::Filtered(d) => &mut d.common, - // } - // } - pub fn id(&self) -> DeckId { self.common().id } @@ -397,11 +393,33 @@ impl From for DeckCommonSchema11 { DeckKind::Normal(n) => n.description, DeckKind::Filtered(_) => String::new(), }, - other: serde_json::from_slice(&deck.common.other).unwrap_or_default(), + other: parse_other_fields(&deck.common.other, &RESERVED_DECK_KEYS), } } } +static RESERVED_DECK_KEYS: Set<&'static str> = phf_set! { + "usn", + "revToday", + "newLimit", + "dyn", + "reviewLimit", + "newToday", + "timeToday", + "reviewLimitToday", + "extendNew", + "mod", + "newLimitToday", + "desc", + "name", + "lrnToday", + "conf", + "browserCollapsed", + "extendRev", + "id", + "collapsed" +}; + impl From<&Deck> for DeckTodaySchema11 { fn from(deck: &Deck) -> Self { let day = deck.common.last_day_studied as i32; @@ -436,3 +454,23 @@ impl From for FilteredSearchTermSchema11 { } } } + +#[cfg(test)] +mod tests { + use itertools::Itertools; + + use super::*; + + #[test] + fn all_reserved_fields_are_removed() -> Result<()> { + let key_source = DeckSchema11::default(); + let mut deck = Deck::new_normal(); + deck.common.other = serde_json::to_vec(&key_source)?; + let DeckSchema11::Normal(s11) = DeckSchema11::from(deck) else { panic!() }; + + let empty: &[&String] = &[]; + assert_eq!(&s11.common.other.keys().collect_vec(), empty); + + Ok(()) + } +} diff --git a/rslib/src/notetype/mod.rs b/rslib/src/notetype/mod.rs index 118f08aef..d2f3f2af6 100644 --- a/rslib/src/notetype/mod.rs +++ b/rslib/src/notetype/mod.rs @@ -8,7 +8,7 @@ mod fields; mod notetypechange; mod render; mod restore; -mod schema11; +pub(crate) mod schema11; mod schemachange; mod service; pub(crate) mod stock; diff --git a/rslib/src/notetype/schema11.rs b/rslib/src/notetype/schema11.rs index ed19b677f..ec61550ac 100644 --- a/rslib/src/notetype/schema11.rs +++ b/rslib/src/notetype/schema11.rs @@ -3,6 +3,8 @@ use std::collections::HashMap; +use phf::phf_set; +use phf::Set; use serde::Deserialize; use serde::Serialize; use serde_json::Value; @@ -127,14 +129,19 @@ fn other_to_bytes(other: &HashMap) -> Vec { } } -fn bytes_to_other(bytes: &[u8]) -> HashMap { +pub(crate) fn parse_other_fields( + bytes: &[u8], + reserved: &Set<&'static str>, +) -> HashMap { if bytes.is_empty() { Default::default() } else { - serde_json::from_slice(bytes).unwrap_or_else(|e| { + let mut map: HashMap = serde_json::from_slice(bytes).unwrap_or_else(|e| { println!("deserialization failed for other: {}", e); Default::default() - }) + }); + map.retain(|k, _v| !reserved.contains(k)); + map } } @@ -165,11 +172,29 @@ impl From for NotetypeSchema11 { latexsvg: c.latex_svg, req: CardRequirementsSchema11(c.reqs.into_iter().map(Into::into).collect()), original_stock_kind: c.original_stock_kind, - other: bytes_to_other(&c.other), + other: parse_other_fields(&c.other, &RESERVED_NOTETYPE_KEYS), } } } +static RESERVED_NOTETYPE_KEYS: Set<&'static str> = phf_set! { + "latexPost", + "flds", + "css", + "originalStockKind", + "id", + "usn", + "mod", + "req", + "latexPre", + "name", + "did", + "tmpls", + "type", + "sortf", + "latexsvg" +}; + impl From for CardRequirement { fn from(r: CardRequirementSchema11) -> Self { CardRequirement { @@ -266,8 +291,6 @@ impl From for NoteField { } } -// fixme: must make sure calling code doesn't break the assumption ord is set - impl From for NoteFieldSchema11 { fn from(p: NoteField) -> Self { let conf = p.config; @@ -282,11 +305,24 @@ impl From for NoteFieldSchema11 { description: conf.description, collapsed: conf.collapsed, exclude_from_search: conf.exclude_from_search, - other: bytes_to_other(&conf.other), + other: parse_other_fields(&conf.other, &RESERVED_FIELD_KEYS), } } } +static RESERVED_FIELD_KEYS: Set<&'static str> = phf_set! { + "name", + "ord", + "sticky", + "rtl", + "plainText", + "font", + "size", + "collapsed", + "description", + "excludeFromSearch", +}; + #[derive(Serialize, Deserialize, Debug, Default, Clone)] pub struct CardTemplateSchema11 { pub(crate) name: String, @@ -329,8 +365,6 @@ impl From for CardTemplate { } } -// fixme: make sure we don't call this when ord not set - impl From for CardTemplateSchema11 { fn from(p: CardTemplate) -> Self { let conf = p.config; @@ -348,7 +382,46 @@ impl From for CardTemplateSchema11 { }, bfont: conf.browser_font_name, bsize: conf.browser_font_size as u8, - other: bytes_to_other(&conf.other), + other: parse_other_fields(&conf.other, &RESERVED_TEMPLATE_KEYS), } } } + +static RESERVED_TEMPLATE_KEYS: Set<&'static str> = phf_set! { + "name", + "ord", + "did", + "afmt", + "bafmt", + "qfmt", + "bqfmt", + "bfont", + "bsize", +}; + +#[cfg(test)] +mod tests { + use itertools::Itertools; + + use super::*; + use crate::notetype::stock::basic; + use crate::prelude::*; + + #[test] + fn all_reserved_fields_are_removed() -> Result<()> { + let mut nt = basic(&I18n::template_only()); + + let key_source = NotetypeSchema11::from(nt.clone()); + nt.config.other = serde_json::to_vec(&key_source)?; + nt.fields[0].config.other = serde_json::to_vec(&key_source.flds[0])?; + nt.templates[0].config.other = serde_json::to_vec(&key_source.tmpls[0])?; + let s11 = NotetypeSchema11::from(nt); + + let empty: &[&String] = &[]; + assert_eq!(&s11.other.keys().collect_vec(), empty); + assert_eq!(&s11.flds[0].other.keys().collect_vec(), empty); + assert_eq!(&s11.tmpls[0].other.keys().collect_vec(), empty); + + Ok(()) + } +} diff --git a/rslib/src/serde.rs b/rslib/src/serde.rs index 50a00bd4e..b0bcfc695 100644 --- a/rslib/src/serde.rs +++ b/rslib/src/serde.rs @@ -7,24 +7,7 @@ pub(crate) use serde_aux::field_attributes::deserialize_bool_from_anything; pub(crate) use serde_aux::field_attributes::deserialize_number_from_string; use serde_json::Value; -use crate::prelude::*; - -/// Serializes the value to JSON, removing any duplicate keys in the process. -/// -/// This function solves a very specific problem when (de)serializing structs on -/// up-/downgrade: -/// Older clients may have received keys from a newer client when -/// syncing, which get bundled into `other`. If they then upgrade, then -/// downgrade their collection to schema11, serde will serialize the -/// new default keys, but then add them again from `other`, leading -/// to the keys being duplicated in the resulting json - which older -/// clients then can't read. So we need to strip out any new keys we -/// add. -pub(crate) fn schema11_to_string(value: impl serde::Serialize) -> Result { - serde_json::to_value(value) - .and_then(|val| serde_json::to_string(&val)) - .map_err(Into::into) -} +use crate::timestamp::TimestampSecs; /// Note: if you wish to cover the case where a field is missing, make sure you /// also use the `serde(default)` flag. diff --git a/rslib/src/storage/deck/mod.rs b/rslib/src/storage/deck/mod.rs index 84e71a2b0..121d32014 100644 --- a/rslib/src/storage/deck/mod.rs +++ b/rslib/src/storage/deck/mod.rs @@ -404,7 +404,7 @@ impl SqliteStorage { pub(crate) fn downgrade_decks_from_schema15(&self) -> Result<()> { let decks = self.get_all_decks_as_schema11()?; - self.set_schema11_decks(&decks) + self.set_schema11_decks(decks) } fn get_schema11_decks(&self) -> Result> { @@ -420,8 +420,8 @@ impl SqliteStorage { Ok(decks) } - pub(crate) fn set_schema11_decks(&self, decks: &HashMap) -> Result<()> { - let json = crate::serde::schema11_to_string(decks)?; + pub(crate) fn set_schema11_decks(&self, decks: HashMap) -> Result<()> { + let json = serde_json::to_string(&decks)?; self.db.execute("update col set decks = ?", [json])?; Ok(()) } diff --git a/rslib/src/storage/deckconfig/mod.rs b/rslib/src/storage/deckconfig/mod.rs index bfa4529bb..1c95234d6 100644 --- a/rslib/src/storage/deckconfig/mod.rs +++ b/rslib/src/storage/deckconfig/mod.rs @@ -255,7 +255,7 @@ impl SqliteStorage { .collect(); self.db.execute( "update col set dconf=?", - params![crate::serde::schema11_to_string(confmap)?], + params![serde_json::to_string(&confmap)?], )?; Ok(()) } diff --git a/rslib/src/storage/notetype/mod.rs b/rslib/src/storage/notetype/mod.rs index 5c3b5ed5b..30625eaff 100644 --- a/rslib/src/storage/notetype/mod.rs +++ b/rslib/src/storage/notetype/mod.rs @@ -361,7 +361,7 @@ impl SqliteStorage { pub(crate) fn downgrade_notetypes_from_schema15(&self) -> Result<()> { let nts = self.get_all_notetypes_as_schema11()?; - self.set_schema11_notetypes(&nts) + self.set_schema11_notetypes(nts) } fn get_schema11_notetypes(&self) -> Result> { @@ -379,9 +379,9 @@ impl SqliteStorage { pub(crate) fn set_schema11_notetypes( &self, - notetypes: &HashMap, + notetypes: HashMap, ) -> Result<()> { - let json = crate::serde::schema11_to_string(notetypes)?; + let json = serde_json::to_string(¬etypes)?; self.db.execute("update col set models = ?", [json])?; Ok(()) }